* [gentoo-catalyst] [PATCH 2/2] catalyst: Replace snakeoil's locks with fasteners
2021-06-10 3:28 [gentoo-catalyst] [PATCH 1/2] catalyst: Remove clear_dir's never used clear_nondir parameter Matt Turner
@ 2021-06-10 3:28 ` Matt Turner
0 siblings, 0 replies; 2+ messages in thread
From: Matt Turner @ 2021-06-10 3:28 UTC (permalink / raw
To: gentoo-catalyst; +Cc: Matt Turner
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.
Signed-off-by: Matt Turner <mattst88@gentoo.org>
---
catalyst/base/clearbase.py | 2 --
catalyst/base/stagebase.py | 10 +++----
catalyst/lock.py | 58 ------------------------------------
catalyst/targets/snapshot.py | 6 ++--
4 files changed, 7 insertions(+), 69 deletions(-)
delete mode 100644 catalyst/lock.py
diff --git a/catalyst/base/clearbase.py b/catalyst/base/clearbase.py
index dcf6c523..6218330e 100644
--- a/catalyst/base/clearbase.py
+++ b/catalyst/base/clearbase.py
@@ -27,12 +27,10 @@ class ClearBase():
self.resume.clear_all(remove=True)
def clear_chroot(self):
- self.chroot_lock.unlock()
log.notice('Clearing the chroot path ...')
clear_dir(self.settings["chroot_path"], mode=0o755)
def remove_chroot(self):
- self.chroot_lock.unlock()
log.notice('Removing the chroot path ...')
clear_dir(self.settings["chroot_path"], mode=0o755, remove=True)
diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index 448d6265..10cffd4c 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -8,6 +8,7 @@ import sys
from pathlib import Path
+import fasteners
import libmount
import toml
@@ -25,7 +26,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, clear_dir, clear_path
from catalyst.base.resume import AutoResume
@@ -36,9 +36,6 @@ def run_sequence(sequence):
sys.stdout.flush()
try:
func()
- except LockInUse:
- log.error('Unable to aquire the lock...')
- return False
except Exception:
log.error('Exception running action sequence %s',
func.__name__, exc_info=True)
@@ -478,7 +475,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(
@@ -1366,8 +1362,10 @@ class StageBase(TargetBase, ClearBase, GenBase):
pass
def run(self):
- self.chroot_lock.write_lock()
+ with fasteners.InterProcessLock(self.settings["chroot_path"] + '.lock'):
+ return self._run()
+ def _run(self):
if "clear-autoresume" in self.settings["options"]:
self.clear_autoresume()
diff --git a/catalyst/lock.py b/catalyst/lock.py
deleted file mode 100644
index e31745b2..00000000
--- a/catalyst/lock.py
+++ /dev/null
@@ -1,58 +0,0 @@
-
-import os
-
-from contextlib import contextmanager
-
-from snakeoil import fileutils
-from snakeoil import osutils
-from catalyst.fileops import ensure_dirs
-
-
-LockInUse = osutils.LockException
-
-class Lock:
- """
- A fnctl-based filesystem lock
- """
- def __init__(self, lockfile):
- fileutils.touch(lockfile, mode=0o664)
- os.chown(lockfile, uid=-1, gid=250)
- self.lock = osutils.FsLock(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()
-
-class LockDir(Lock):
- """
- A fnctl-based filesystem lock in a directory
- """
- def __init__(self, lockdir):
- ensure_dirs(lockdir)
- lockfile = os.path.join(lockdir, '.catalyst_lock')
-
- Lock.__init__(self, lockfile)
-
-@contextmanager
-def read_lock(filename):
- lock = Lock(filename)
- lock.read_lock()
- try:
- yield
- finally:
- lock.unlock()
-
-@contextmanager
-def write_lock(filename):
- lock = Lock(filename)
- lock.write_lock()
- try:
- yield
- finally:
- lock.unlock()
diff --git a/catalyst/targets/snapshot.py b/catalyst/targets/snapshot.py
index b494575a..ef68765d 100644
--- a/catalyst/targets/snapshot.py
+++ b/catalyst/targets/snapshot.py
@@ -5,11 +5,12 @@ Snapshot target
import subprocess
import sys
+import fasteners
+
from pathlib import Path
from catalyst import log
from catalyst.base.targetbase import TargetBase
-from catalyst.lock import write_lock
from catalyst.support import CatalystError, command
class snapshot(TargetBase):
@@ -93,8 +94,7 @@ class snapshot(TargetBase):
log.notice('>>> ' + ' '.join([*git_cmd, '|']))
log.notice(' ' + ' '.join(tar2sqfs_cmd))
- lockfile = self.snapshot.with_suffix('.lock')
- with write_lock(lockfile):
+ with fasteners.InterProcessLock(self.snapshot.with_suffix('.lock')):
git = subprocess.Popen(git_cmd,
stdout=subprocess.PIPE,
stderr=sys.stderr,
--
2.31.1
^ permalink raw reply related [flat|nested] 2+ messages in thread