From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id 09EFF1382C5 for ; Thu, 10 Jun 2021 03:28:50 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 2600FE075F; Thu, 10 Jun 2021 03:28:49 +0000 (UTC) Received: from mail-pg1-f172.google.com (mail-pg1-f172.google.com [209.85.215.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 1C13CE075F for ; Thu, 10 Jun 2021 03:28:48 +0000 (UTC) Received: by mail-pg1-f172.google.com with SMTP id t9so21433483pgn.4 for ; Wed, 09 Jun 2021 20:28:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=tJ0RT1J8T3CuKpuU6Ii9k49VdKvvCGG2ezyaOOO3bFA=; b=hTx6yY2eZhahSH7qcceWoUbyx5KrDQgVD9XP5mOKyxpcNcKC+wlgHcULp/P7vfZ5Md nrqEgbdO6CtlpB/KdSNhi5Zgd2j17gBqZQBTsoLSOLfmyL9LrMSpMUcHTSbUPypJnZqe vdouKvMazxyw82Jl53LOq/OSafc7oWIPoLrWqmQ8O/NEgZ1vDED4DqPlpxJfcL5BXm7S VH4/eRRmO5x/p8HqYDtRzBYcAU9mjFkWZvrAqQZMWxG1G9HIOM3Y0972vyqb5E+j8kaz RSG2oLZB1nJOcPh0W3e8yffitQIAXPZVCuyh5b76ST+g0klXtJqBqnJGdI867wsRE0qO xU1A== X-Gm-Message-State: AOAM530kY+2mq2mWNzdXzvBlNC7FnpwaT4tREtK1yfghFwtzJaUIDWDp wVCLqHyalzklWX21aTRw+E/fhHgzLZk= X-Google-Smtp-Source: ABdhPJyUMUPt21Z3vvhmOj2QxWC/OOuEqIaQ3frYzbLRmQbkEoqDlx0BzMI0+4F5p484Qx4HcF58Rw== X-Received: by 2002:a63:7702:: with SMTP id s2mr2907356pgc.106.1623295727495; Wed, 09 Jun 2021 20:28:47 -0700 (PDT) Received: from localhost ([108.161.26.224]) by smtp.gmail.com with ESMTPSA id 22sm787105pfo.80.2021.06.09.20.28.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Jun 2021 20:28:47 -0700 (PDT) From: Matt Turner To: gentoo-catalyst@lists.gentoo.org Cc: Matt Turner Subject: [gentoo-catalyst] [PATCH 2/2] catalyst: Replace snakeoil's locks with fasteners Date: Wed, 9 Jun 2021 20:28:40 -0700 Message-Id: <20210610032840.466809-2-mattst88@gentoo.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210610032840.466809-1-mattst88@gentoo.org> References: <20210610032840.466809-1-mattst88@gentoo.org> Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-catalyst@lists.gentoo.org Reply-to: gentoo-catalyst@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Archives-Salt: 4cd9ca85-fed6-44d4-b446-8601ed542d22 X-Archives-Hash: be1d4987ac84f0f774f6bbc0e5d1b4e6 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 --- 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