From 798ebbe7fa9d3c464df70d2db95ad721b24b20bd Mon Sep 17 00:00:00 2001 From: chchatzop <35049131+chchatzop@users.noreply.github.com> Date: Wed, 23 Sep 2026 01:13:09 +0300 Subject: [PATCH] The library-scan locks live in runtime.py, and the lock guard sees the or form (#749) dcc.py built its scan semaphore and lookup-memory lock as `x = globals().get("x") or threading.Lock()`, against the rule that a reloaded module never constructs its own lock; the guard missed them because the factory call sat inside a BoolOp. runtime.py owns both now, bound by name in dcc.py; the memories stay dcc.py's own cache. The guard walks the whole value, its control gains the or / conditional forms and a plain `or {}` that must not be flagged, and two tests check the binding and the semaphore's size. No behaviour change. Dev changelog. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01AP6LSxkr4n9dMFNSNMogmW --- dcc.py | 9 ++-- docs/UPDATES.md | 17 ++++++ runtime.py | 10 ++++ tests/test_no_reloaded_module_owns_a_lock.py | 56 +++++++++++++++++--- 4 files changed, 82 insertions(+), 10 deletions(-) diff --git a/dcc.py b/dcc.py index 63a48b3b..e029eb52 100644 --- a/dcc.py +++ b/dcc.py @@ -73,7 +73,7 @@ def names_a_remote_or_absolute_path(name, windows=None): # is busy, at once, without touching the disk), and a name that just missed is # answered "not found" from memory for a minute - the same stale row pasted ten # times costs one scan, not ten. -MAX_CONCURRENT_LIBRARY_SCANS = 2 +MAX_CONCURRENT_LIBRARY_SCANS = runtime.MAX_CONCURRENT_LIBRARY_SCANS LOOKUP_MISS_TTL_SECONDS = 60.0 LOOKUP_MISS_MEMORY = 512 # WHAT A PASTE OF NINE ROWS COSTS (#886). Only misses were remembered, so @@ -94,9 +94,12 @@ def names_a_remote_or_absolute_path(name, windows=None): LOOKUP_HIT_MEMORY = 512 LOOKUP_FOLDER_MEMORY = 32 LOOKUP_SCAN_WAIT_SECONDS = 5.0 -_library_scans = globals().get("_library_scans") or threading.BoundedSemaphore(MAX_CONCURRENT_LIBRARY_SCANS) +# Owned by runtime.py, which nothing reloads (#749) - bound by name, the way +# queue_lock is, so a !rehash re-runs these lines and picks the same live +# objects back up. The memories themselves are this module's own cache. +_library_scans = runtime.library_scans _lookup_misses = globals().get("_lookup_misses") or {} -_lookup_misses_lock = globals().get("_lookup_misses_lock") or threading.Lock() +_lookup_misses_lock = runtime.lookup_memory_lock _lookup_hits = globals().get("_lookup_hits") or {} _lookup_folders = globals().get("_lookup_folders") or {} diff --git a/docs/UPDATES.md b/docs/UPDATES.md index dec665f2..e4dcbbd2 100644 --- a/docs/UPDATES.md +++ b/docs/UPDATES.md @@ -4,6 +4,23 @@ All version changes, optimizations, and bug fixes made over time in the DCCore p ## 🟨 Unreleased +### 🔒 The library-scan locks live in runtime.py, and the lock guard sees the `or` form (#749) + +Follow-up to #731 (#580). `dcc.py` built its scan semaphore and its lookup-memory lock at module level as +`x = globals().get("x") or threading.Lock()` - kept across a `!rehash` only for as long as the old object is +found - while the repository's rule (#235) is that a module `!rehash` reloads never constructs its own lock: +`runtime.py`, which nothing reloads, owns them and the module binds the name. `tests/test_no_reloaded_module_owns_a_lock.py` +could not see the pair: it looked for a factory call as the *whole* right-hand side, and here the call sits +inside a `BoolOp`. Every lock test stayed green. + +`runtime.library_scans` (with `MAX_CONCURRENT_LIBRARY_SCANS`) and `runtime.lookup_memory_lock` now, bound by +name in `dcc.py` like `queue_lock`. The memories they protect stay in `dcc.py` - that module's own cache, not the +configuration state `runtime.py`'s containers are (`test_runtime_state` binds and resets those). The guard walks the +whole value (`constructs_a_lock()`), so a factory called inside `or`, a conditional or an argument is caught; its +synthetic control gains the `or` and conditional forms and a plain `or {}`, which must not be flagged. Run before +the move, the extended guard names exactly the two; `TheLookupLocksAreBoundFromRuntime` checks the binding and that +the semaphore admits exactly the scans `dcc` says. No behaviour change. + ### 🔧 Re-running setup no longer collapses more than one services host to just the first (#891) Found reviewing #811's merged change. A blank answer at the services-host prompt showed the first configured diff --git a/runtime.py b/runtime.py index 6d1529fb..f0cbedd4 100644 --- a/runtime.py +++ b/runtime.py @@ -132,6 +132,16 @@ disk_lock = threading.Lock() # db.py's serialised on-disk writes told_queue_full_lock = threading.Lock() # announce.py's queue-full notice memory (#888) +# dcc.py's library lookup (#580, #886), moved here in #749. They were built in +# dcc.py as `x = globals().get("x") or threading.Lock()` - kept across a reload +# only for as long as the old object is found - and the lock guard could not +# see the `or` form (it now can). The memories they protect stay in dcc.py: +# they are that module's own cache, not the configuration state the +# containers in this file are. +MAX_CONCURRENT_LIBRARY_SCANS = 2 +library_scans = threading.BoundedSemaphore(MAX_CONCURRENT_LIBRARY_SCANS) # library scans at once +lookup_memory_lock = threading.Lock() # dcc.py's lookup memories - misses, hits, folders + # The reload window, which is not only about rebinding. # # importlib.reload(defaults) re-executes defaults.py from the top, and that diff --git a/tests/test_no_reloaded_module_owns_a_lock.py b/tests/test_no_reloaded_module_owns_a_lock.py index 4787069d..f4710259 100644 --- a/tests/test_no_reloaded_module_owns_a_lock.py +++ b/tests/test_no_reloaded_module_owns_a_lock.py @@ -50,6 +50,27 @@ LOCK_FACTORIES = {"Lock", "RLock", "Condition", "Semaphore", "BoundedSemaphore"} +def constructs_a_lock(value): + """True when a lock factory is CALLED anywhere in an assignment's value. + + Not only as the whole right-hand side (#749). The shape this missed was + `x = globals().get("x") or threading.Lock()` - the call sits inside a + BoolOp, so a scan that looked only at the top node read it as a plain + expression. Two of them were in dcc.py, rebuilt by `or` on any reload + whose globals().get came back empty, and every lock test stayed green. + Walking the value finds a factory call however it is wrapped: `or`, + a conditional, an argument. + """ + for node in ast.walk(value): + if isinstance(node, ast.Call): + func = node.func + name = (func.attr if isinstance(func, ast.Attribute) + else getattr(func, "id", None)) + if name in LOCK_FACTORIES: + return True + return False + + def module_level_locks(): """(module, line, name) for every module-level lock object constructed.""" found = [] @@ -68,12 +89,7 @@ def module_level_locks(): targets, value = [node.target], node.value else: continue - if not isinstance(value, ast.Call): - continue - func = value.func - name = (func.attr if isinstance(func, ast.Attribute) - else getattr(func, "id", None)) - if name not in LOCK_FACTORIES: + if not constructs_a_lock(value): continue for target in targets: if isinstance(target, ast.Name): @@ -132,6 +148,10 @@ def test_it_recognises_a_lock_however_it_is_spelled(self): "c = threading.RLock()\n" "d: object = threading.Condition()\n" "e = 5\n" + "g = globals().get('g') or threading.Lock()\n" + "h = globals().get('h') or threading.BoundedSemaphore(2)\n" + "i = threading.Lock() if e else None\n" + "j = globals().get('j') or {}\n" "def f():\n" " local = threading.Lock()\n") import tempfile @@ -145,7 +165,7 @@ def test_it_recognises_a_lock_however_it_is_spelled(self): mine = sorted(name for module, _line, name in module_level_locks() if module == os.path.basename(path)[:-3]) - self.assertEqual(mine, ["a", "b", "c", "d"], + self.assertEqual(mine, ["a", "b", "c", "d", "g", "h", "i"], "a lock spelling is unrecognised, or a function-local " "one is being reported") @@ -163,5 +183,27 @@ def test_a_lock_added_to_a_reloaded_module_would_be_caught(self): "as a problem, so this guard would not catch one") + +class TheLookupLocksAreBoundFromRuntime(unittest.TestCase): + """#749: dcc.py's scan semaphore and lookup-memory lock were the two the + or-form hid. Bound by name now, like list_mod._count_lock.""" + + def test_they_are_runtimes_objects(self): + import dcc + import runtime + self.assertIs(dcc._library_scans, runtime.library_scans) + self.assertIs(dcc._lookup_misses_lock, runtime.lookup_memory_lock) + + def test_the_semaphore_admits_exactly_the_scans_dcc_says(self): + import dcc + held = 0 + try: + while held <= dcc.MAX_CONCURRENT_LIBRARY_SCANS and dcc._library_scans.acquire(blocking=False): + held += 1 + self.assertEqual(held, dcc.MAX_CONCURRENT_LIBRARY_SCANS) + finally: + for _ in range(held): + dcc._library_scans.release() + if __name__ == "__main__": unittest.main()