[gentoo-commits] proj/catalyst:catalyst-3.0-stable commit in: catalyst/, catalyst/base/

[Matt Turner]

commit:     9221e327aecfd8585cbb9796add9224123d1c53a
Author:     Matt Turner <mattst88 <AT> gentoo <DOT> org>
AuthorDate: Wed Jun  9 06:17:31 2021 +0000
Commit:     Matt Turner <mattst88 <AT> gentoo <DOT> org>
CommitDate: Fri Jun 11 04:10:12 2021 +0000
URL:        https://gitweb.gentoo.org/proj/catalyst.git/commit/?id=9221e327

catalyst: Replace snakeoil's locks with fasteners

To no great surprise, the existing locking was broken. For example,
clear_chroot() releases the lock. It is called by unpack(), which is
part of prepare_sequence. The result is that the whole build could be
done without holding the lock.

Just lock around run(). It's not apparent that finer-grained locking
does anything for us.

For the catalyst-3.0-stable branch, just remove all the snapcache
locking. It's broken (bug #519656) and no one is going to fix it, since
snapcache has been removed from the master branch.

Bug: https://bugs.gentoo.org/791583
Closes: https://bugs.gentoo.org/519656
Signed-off-by: Matt Turner <mattst88 <AT> gentoo.org>
(cherry picked from commit b3c917f7fa73d11c69b7e55dc7a00bc18a18edc7)

 catalyst/base/clearbase.py |  2 --
 catalyst/base/stagebase.py | 26 +++++---------------------
 catalyst/lock.py           | 31 -------------------------------
 3 files changed, 5 insertions(+), 54 deletions(-)

diff --git a/catalyst/base/clearbase.py b/catalyst/base/clearbase.py
index 644a385f..388b1225 100644
--- a/catalyst/base/clearbase.py
+++ b/catalyst/base/clearbase.py
@@ -28,13 +28,11 @@ class ClearBase(object):
 
 
 	def clear_chroot(self):
-		self.chroot_lock.unlock()
 		log.notice('Clearing the chroot path ...')
 		clear_dir(self.settings["chroot_path"], mode=0o755, chg_flags=True)
 
 
 	def remove_chroot(self):
-		self.chroot_lock.unlock()
 		log.notice('Removing the chroot path ...')
 		clear_dir(self.settings["chroot_path"], mode=0o755, chg_flags=True, remove=True)
 

diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index 770d1b35..ad0028e1 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -4,6 +4,8 @@ import imp
 import shutil
 import sys
 
+import fasteners
+
 from snakeoil import fileutils
 
 from DeComp.compress import CompressMap
@@ -16,7 +18,6 @@ from catalyst.support import (CatalystError, file_locate, normpath,
 from catalyst.base.targetbase import TargetBase
 from catalyst.base.clearbase import ClearBase
 from catalyst.base.genbase import GenBase
-from catalyst.lock import LockDir, LockInUse
 from catalyst.fileops import ensure_dirs, pjoin, clear_dir, clear_path
 from catalyst.base.resume import AutoResume
 
@@ -496,8 +497,6 @@ class StageBase(TargetBase, ClearBase, GenBase):
 			self.settings["snapshot_cache_path"] = \
 				normpath(pjoin(self.settings["snapshot_cache"],
 					self.settings["snapshot"]))
-			self.snapcache_lock = \
-				LockDir(self.settings["snapshot_cache_path"])
 			log.info('Setting snapshot cache to %s', self.settings['snapshot_cache_path'])
 
 	def set_chroot_path(self):
@@ -507,7 +506,6 @@ class StageBase(TargetBase, ClearBase, GenBase):
 		"""
 		self.settings["chroot_path"] = normpath(self.settings["storedir"] +
 			"/tmp/" + self.settings["target_subpath"].rstrip('/'))
-		self.chroot_lock = LockDir(self.settings["chroot_path"])
 
 	def set_autoresume_path(self):
 		self.settings["autoresume_path"] = normpath(pjoin(
@@ -881,8 +879,6 @@ class StageBase(TargetBase, ClearBase, GenBase):
 				unpack = False
 
 		if unpack:
-			if "snapcache" in self.settings["options"]:
-				self.snapcache_lock.write_lock()
 			if os.path.exists(target_portdir):
 				log.info('%s', cleanup_msg)
 			clear_dir(target_portdir)
@@ -899,9 +895,6 @@ class StageBase(TargetBase, ClearBase, GenBase):
 				self.resume.enable("unpack_repo",
 					data = self.settings["snapshot_path_hash"])
 
-			if "snapcache" in self.settings["options"]:
-				self.snapcache_lock.unlock()
-
 	def config_profile_link(self):
 		if "autoresume" in self.settings["options"] \
 			and self.resume.is_enabled("config_profile_link"):
@@ -971,8 +964,6 @@ class StageBase(TargetBase, ClearBase, GenBase):
 
 			src = self.mountmap[x]
 			log.debug('bind(); src = %s', src)
-			if "snapcache" in self.settings["options"] and x == "portdir":
-				self.snapcache_lock.read_lock()
 			_cmd = None
 			if src == "maybe_tmpfs":
 				if "var_tmpfs_portage" in self.settings:
@@ -1026,15 +1017,6 @@ class StageBase(TargetBase, ClearBase, GenBase):
 				except CatalystError:
 					ouch = 1
 					log.warning("Couldn't umount bind mount: %s", target)
-
-			if "snapcache" in self.settings["options"] and x == "/var/db/repos/gentoo":
-				try:
-					# It's possible the snapshot lock object isn't created yet.
-					# This is because mount safety check calls unbind before the
-					# target is fully initialized
-					self.snapcache_lock.unlock()
-				except Exception:
-					pass
 		if ouch:
 			# if any bind mounts really failed, then we need to raise
 			# this to potentially prevent an upcoming bash stage cleanup script
@@ -1484,8 +1466,10 @@ class StageBase(TargetBase, ClearBase, GenBase):
 		log.debug('setup_environment(); env = %r', self.env)
 
 	def run(self):
-		self.chroot_lock.write_lock()
+		with fasteners.InterProcessLock(self.settings["chroot_path"] + '.lock'):
+			return self._run()
 
+	def _run(self):
 		# Kill any pids in the chroot
 		self.kill_chroot_pids()
 

diff --git a/catalyst/lock.py b/catalyst/lock.py
deleted file mode 100644
index 808df4ec..00000000
--- a/catalyst/lock.py
+++ /dev/null
@@ -1,31 +0,0 @@
-
-import os
-
-from snakeoil import fileutils
-from snakeoil import osutils
-from catalyst.fileops import ensure_dirs
-
-
-LockInUse = osutils.LockException
-
-
-class LockDir(object):
-	"""An object that creates locks inside dirs"""
-
-	def __init__(self, lockdir):
-		self.gid = 250
-		self.lockfile = os.path.join(lockdir, '.catalyst_lock')
-		ensure_dirs(lockdir)
-		fileutils.touch(self.lockfile, mode=0o664)
-		os.chown(self.lockfile, -1, self.gid)
-		self.lock = osutils.FsLock(self.lockfile)
-
-	def read_lock(self):
-		self.lock.acquire_read_lock()
-
-	def write_lock(self):
-		self.lock.acquire_write_lock()
-
-	def unlock(self):
-		# Releasing a write lock is the same as a read lock.
-		self.lock.release_write_lock()