diff --git a/frappe/installer.py b/frappe/installer.py index 0cd9b32063..e78112df19 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -16,6 +16,7 @@ import frappe from frappe.defaults import _clear_cache from frappe.utils import cint, is_git_url from frappe.utils.dashboard import sync_dashboards +from frappe.utils.synchronization import filelock def _is_scheduler_enabled() -> bool: @@ -540,8 +541,11 @@ def make_site_config( f.write(json.dumps(site_config, indent=1, sort_keys=True)) +@filelock("site_config") def update_site_config(key, value, validate=True, site_config_path=None): """Update a value in site_config""" + from frappe.utils.synchronization import filelock + if not site_config_path: site_config_path = get_site_config_path() diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 44966691a0..cf6134d101 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -68,6 +68,7 @@ from frappe.utils.identicon import Identicon from frappe.utils.image import optimize_image, strip_exif_data from frappe.utils.make_random import can_make, get_random, how_many from frappe.utils.response import json_handler +from frappe.utils.synchronization import LockTimeoutError, filelock class Capturing(list): @@ -880,6 +881,22 @@ class TestContainerUtils(FrappeTestCase): self.assertEqual(a["c"], "d") +class TestLocks(FrappeTestCase): + def test_locktimeout(self): + lock_name = "test_lock" + with filelock(lock_name): + with self.assertRaises(LockTimeoutError): + with filelock(lock_name, timeout=1): + self.fail("Locks not working") + + def test_global_lock(self): + lock_name = "test_global" + with filelock(lock_name, is_global=True): + with self.assertRaises(LockTimeoutError): + with filelock(lock_name, timeout=1, is_global=True): + self.fail("Global locks not working") + + class TestMiscUtils(FrappeTestCase): def test_get_file_timestamp(self): self.assertIsInstance(get_file_timestamp(__file__), str) diff --git a/frappe/utils/file_lock.py b/frappe/utils/file_lock.py index fc399f7c96..5be89c42f6 100644 --- a/frappe/utils/file_lock.py +++ b/frappe/utils/file_lock.py @@ -1,22 +1,34 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE -""" -File based locking utility +"""Utils for inter-process synchronization using file-locks. + +This file implements a "weak" form lock which is not suitable for synchroniztion. This is only used +for document locking for queue_action. +Use `frappe.utils.synchroniztion.filelock` for process synchroniztion. """ import os from time import time +from frappe import _ from frappe.utils import get_site_path, touch_file +LOCKS_DIR = "locks" + class LockTimeoutError(Exception): pass def create_lock(name): - """Creates a file in the /locks folder by the given name""" + """Creates a file in the /locks folder by the given name. + + Note: This is a "weak lock" and is prone to race conditions. Do not use this lock for small + sections of code that execute immediately. + + This is primarily use for locking documents for background submission. + """ lock_path = get_lock_path(name) if not check_lock(lock_path): return touch_file(lock_path) @@ -48,6 +60,5 @@ def delete_lock(name): def get_lock_path(name): name = name.lower() - locks_dir = "locks" - lock_path = get_site_path(locks_dir, name + ".lock") + lock_path = get_site_path(LOCKS_DIR, name + ".lock") return lock_path diff --git a/frappe/utils/synchronization.py b/frappe/utils/synchronization.py new file mode 100644 index 0000000000..841fab85ff --- /dev/null +++ b/frappe/utils/synchronization.py @@ -0,0 +1,49 @@ +""" Utils for thread/process synchronization. """ + +import os +from contextlib import contextmanager + +from filelock import FileLock as _StrongFileLock +from filelock import Timeout + +import frappe +from frappe import _ +from frappe.utils import get_bench_path, get_site_path +from frappe.utils.file_lock import LockTimeoutError + +LOCKS_DIR = "locks" + + +@contextmanager +def filelock(lock_name: str, *, timeout=30, is_global=False): + """Create a lockfile to prevent concurrent operations acrosss processes. + + args: + lock_name: Unique name to identify a specific lock. Lockfile called `{name}.lock` will be + created. + timeout: time to wait before failing. + is_global: if set lock is global to bench + + Lock file location: + global - {bench_dir}/config/{name}.lock + site - {bench_dir}/sites/sitename/{name}.lock + + """ + + lock_filename = lock_name + ".lock" + if not is_global: + lock_path = os.path.abspath(get_site_path(LOCKS_DIR, lock_filename)) + else: + lock_path = os.path.abspath(os.path.join(get_bench_path(), "config", lock_filename)) + + try: + with _StrongFileLock(lock_path, timeout=timeout): + yield + except Timeout as e: + frappe.log_error("Filelock: Failed to aquire {lock_path}") + + raise LockTimeoutError( + _("Failed to aquire lock: {}").format(lock_name) + + "
" + + _("You can manually remove the lock if you think it's safe: {}").format(lock_path) + ) from e diff --git a/pyproject.toml b/pyproject.toml index f5ab8c94b2..dcd7599e9a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,6 +11,7 @@ dependencies = [ # core dependencies "Babel~=2.9.0", "Click~=7.1.2", + "filelock~=3.8.0", "GitPython~=3.1.14", "Jinja2~=3.1.2", "Pillow~=9.3.0",