1 |
commit: b3c917f7fa73d11c69b7e55dc7a00bc18a18edc7 |
2 |
Author: Matt Turner <mattst88 <AT> gentoo <DOT> org> |
3 |
AuthorDate: Wed Jun 9 06:17:31 2021 +0000 |
4 |
Commit: Matt Turner <mattst88 <AT> gentoo <DOT> org> |
5 |
CommitDate: Thu Jun 10 04:23:54 2021 +0000 |
6 |
URL: https://gitweb.gentoo.org/proj/catalyst.git/commit/?id=b3c917f7 |
7 |
|
8 |
catalyst: Replace snakeoil's locks with fasteners |
9 |
|
10 |
To no great surprise, the existing locking was broken. For example, |
11 |
clear_chroot() releases the lock. It is called by unpack(), which is |
12 |
part of prepare_sequence. The result is that the whole build could be |
13 |
done without holding the lock. |
14 |
|
15 |
Just lock around run(). It's not apparent that finer-grained locking |
16 |
does anything for us. |
17 |
|
18 |
Bug: https://bugs.gentoo.org/791583 |
19 |
Signed-off-by: Matt Turner <mattst88 <AT> gentoo.org> |
20 |
|
21 |
catalyst/base/clearbase.py | 2 -- |
22 |
catalyst/base/stagebase.py | 10 +++----- |
23 |
catalyst/lock.py | 58 -------------------------------------------- |
24 |
catalyst/targets/snapshot.py | 6 ++--- |
25 |
4 files changed, 7 insertions(+), 69 deletions(-) |
26 |
|
27 |
diff --git a/catalyst/base/clearbase.py b/catalyst/base/clearbase.py |
28 |
index dcf6c523..6218330e 100644 |
29 |
--- a/catalyst/base/clearbase.py |
30 |
+++ b/catalyst/base/clearbase.py |
31 |
@@ -27,12 +27,10 @@ class ClearBase(): |
32 |
self.resume.clear_all(remove=True) |
33 |
|
34 |
def clear_chroot(self): |
35 |
- self.chroot_lock.unlock() |
36 |
log.notice('Clearing the chroot path ...') |
37 |
clear_dir(self.settings["chroot_path"], mode=0o755) |
38 |
|
39 |
def remove_chroot(self): |
40 |
- self.chroot_lock.unlock() |
41 |
log.notice('Removing the chroot path ...') |
42 |
clear_dir(self.settings["chroot_path"], mode=0o755, remove=True) |
43 |
|
44 |
|
45 |
diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py |
46 |
index 448d6265..10cffd4c 100644 |
47 |
--- a/catalyst/base/stagebase.py |
48 |
+++ b/catalyst/base/stagebase.py |
49 |
@@ -8,6 +8,7 @@ import sys |
50 |
|
51 |
from pathlib import Path |
52 |
|
53 |
+import fasteners |
54 |
import libmount |
55 |
import toml |
56 |
|
57 |
@@ -25,7 +26,6 @@ from catalyst.support import (CatalystError, file_locate, normpath, |
58 |
from catalyst.base.targetbase import TargetBase |
59 |
from catalyst.base.clearbase import ClearBase |
60 |
from catalyst.base.genbase import GenBase |
61 |
-from catalyst.lock import LockDir, LockInUse |
62 |
from catalyst.fileops import ensure_dirs, clear_dir, clear_path |
63 |
from catalyst.base.resume import AutoResume |
64 |
|
65 |
@@ -36,9 +36,6 @@ def run_sequence(sequence): |
66 |
sys.stdout.flush() |
67 |
try: |
68 |
func() |
69 |
- except LockInUse: |
70 |
- log.error('Unable to aquire the lock...') |
71 |
- return False |
72 |
except Exception: |
73 |
log.error('Exception running action sequence %s', |
74 |
func.__name__, exc_info=True) |
75 |
@@ -478,7 +475,6 @@ class StageBase(TargetBase, ClearBase, GenBase): |
76 |
""" |
77 |
self.settings["chroot_path"] = normpath(self.settings["storedir"] + |
78 |
"/tmp/" + self.settings["target_subpath"].rstrip('/')) |
79 |
- self.chroot_lock = LockDir(self.settings["chroot_path"]) |
80 |
|
81 |
def set_autoresume_path(self): |
82 |
self.settings["autoresume_path"] = normpath(pjoin( |
83 |
@@ -1366,8 +1362,10 @@ class StageBase(TargetBase, ClearBase, GenBase): |
84 |
pass |
85 |
|
86 |
def run(self): |
87 |
- self.chroot_lock.write_lock() |
88 |
+ with fasteners.InterProcessLock(self.settings["chroot_path"] + '.lock'): |
89 |
+ return self._run() |
90 |
|
91 |
+ def _run(self): |
92 |
if "clear-autoresume" in self.settings["options"]: |
93 |
self.clear_autoresume() |
94 |
|
95 |
|
96 |
diff --git a/catalyst/lock.py b/catalyst/lock.py |
97 |
deleted file mode 100644 |
98 |
index e31745b2..00000000 |
99 |
--- a/catalyst/lock.py |
100 |
+++ /dev/null |
101 |
@@ -1,58 +0,0 @@ |
102 |
- |
103 |
-import os |
104 |
- |
105 |
-from contextlib import contextmanager |
106 |
- |
107 |
-from snakeoil import fileutils |
108 |
-from snakeoil import osutils |
109 |
-from catalyst.fileops import ensure_dirs |
110 |
- |
111 |
- |
112 |
-LockInUse = osutils.LockException |
113 |
- |
114 |
-class Lock: |
115 |
- """ |
116 |
- A fnctl-based filesystem lock |
117 |
- """ |
118 |
- def __init__(self, lockfile): |
119 |
- fileutils.touch(lockfile, mode=0o664) |
120 |
- os.chown(lockfile, uid=-1, gid=250) |
121 |
- self.lock = osutils.FsLock(lockfile) |
122 |
- |
123 |
- def read_lock(self): |
124 |
- self.lock.acquire_read_lock() |
125 |
- |
126 |
- def write_lock(self): |
127 |
- self.lock.acquire_write_lock() |
128 |
- |
129 |
- def unlock(self): |
130 |
- # Releasing a write lock is the same as a read lock. |
131 |
- self.lock.release_write_lock() |
132 |
- |
133 |
-class LockDir(Lock): |
134 |
- """ |
135 |
- A fnctl-based filesystem lock in a directory |
136 |
- """ |
137 |
- def __init__(self, lockdir): |
138 |
- ensure_dirs(lockdir) |
139 |
- lockfile = os.path.join(lockdir, '.catalyst_lock') |
140 |
- |
141 |
- Lock.__init__(self, lockfile) |
142 |
- |
143 |
-@contextmanager |
144 |
-def read_lock(filename): |
145 |
- lock = Lock(filename) |
146 |
- lock.read_lock() |
147 |
- try: |
148 |
- yield |
149 |
- finally: |
150 |
- lock.unlock() |
151 |
- |
152 |
-@contextmanager |
153 |
-def write_lock(filename): |
154 |
- lock = Lock(filename) |
155 |
- lock.write_lock() |
156 |
- try: |
157 |
- yield |
158 |
- finally: |
159 |
- lock.unlock() |
160 |
|
161 |
diff --git a/catalyst/targets/snapshot.py b/catalyst/targets/snapshot.py |
162 |
index b494575a..ef68765d 100644 |
163 |
--- a/catalyst/targets/snapshot.py |
164 |
+++ b/catalyst/targets/snapshot.py |
165 |
@@ -5,11 +5,12 @@ Snapshot target |
166 |
import subprocess |
167 |
import sys |
168 |
|
169 |
+import fasteners |
170 |
+ |
171 |
from pathlib import Path |
172 |
|
173 |
from catalyst import log |
174 |
from catalyst.base.targetbase import TargetBase |
175 |
-from catalyst.lock import write_lock |
176 |
from catalyst.support import CatalystError, command |
177 |
|
178 |
class snapshot(TargetBase): |
179 |
@@ -93,8 +94,7 @@ class snapshot(TargetBase): |
180 |
log.notice('>>> ' + ' '.join([*git_cmd, '|'])) |
181 |
log.notice(' ' + ' '.join(tar2sqfs_cmd)) |
182 |
|
183 |
- lockfile = self.snapshot.with_suffix('.lock') |
184 |
- with write_lock(lockfile): |
185 |
+ with fasteners.InterProcessLock(self.snapshot.with_suffix('.lock')): |
186 |
git = subprocess.Popen(git_cmd, |
187 |
stdout=subprocess.PIPE, |
188 |
stderr=sys.stderr, |