From da78ee036aa952aee60476eeb97128e5658172b7 Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Sat, 3 Oct 2026 21:37:21 +0200 Subject: [PATCH 1/5] fix(python): share bounded reentrant lock stripes --- src/Fable.Build/Test/Python.fs | 4 ++ src/fable-library-py/fable_library/util.py | 12 ++-- tests/Python/test_lock_runtime.py | 72 ++++++++++++++++++++++ 3 files changed, 82 insertions(+), 6 deletions(-) create mode 100644 tests/Python/test_lock_runtime.py diff --git a/src/Fable.Build/Test/Python.fs b/src/Fable.Build/Test/Python.fs index 87081b3596..caaaffb692 100644 --- a/src/Fable.Build/Test/Python.fs +++ b/src/Fable.Build/Test/Python.fs @@ -23,6 +23,10 @@ let handle (args: string list) = Directory.clean buildDir + // Native runtime tests use controlled timers and real threads alongside generated F# regressions. + for testFile in Directory.GetFiles(sourceDir, "test_*.py") do + File.Copy(testFile, Path.Combine(buildDir, Path.GetFileName(testFile)), overwrite = true) + Command.Run("uv", "sync") // Install local fable-library as editable package for testing diff --git a/src/fable-library-py/fable_library/util.py b/src/fable-library-py/fable_library/util.py index c2abf502e5..d6797f14f7 100644 --- a/src/fable-library-py/fable_library/util.py +++ b/src/fable-library-py/fable_library/util.py @@ -354,13 +354,13 @@ def assert_not_equal[T](actual: T, expected: T, msg: str | None = None) -> None: MAX_LOCKS = 1024 -def lock[T](lock_obj: Any, fn: Callable[[], T]) -> T: - @functools.lru_cache(maxsize=MAX_LOCKS) - def get_lock(n: int) -> RLock: - return RLock() +# decision: fixed stripes bound synchronization storage without evicting a lock held by another thread +# tradeoff: unrelated objects can share a stripe and serialize, but object identity always selects the same lock +_locks = tuple(RLock() for _ in range(MAX_LOCKS)) + - lock = get_lock(id(lock_obj)) - with lock: +def lock[T](lock_obj: Any, fn: Callable[[], T]) -> T: + with _locks[(id(lock_obj) >> 4) % MAX_LOCKS]: return fn() diff --git a/tests/Python/test_lock_runtime.py b/tests/Python/test_lock_runtime.py new file mode 100644 index 0000000000..37a31cc760 --- /dev/null +++ b/tests/Python/test_lock_runtime.py @@ -0,0 +1,72 @@ +from threading import Event, RLock, Thread + +import pytest +from fable_library import util + + +class ObservedLock: + def __init__(self, observed: Event): + self.lock = RLock() + self.observed = observed + + def __enter__(self): + acquired = self.lock.acquire(blocking=False) + self.observed.set() + if not acquired: + self.lock.acquire() + return self + + def __exit__(self, *args): + self.lock.release() + + +def test_real_threads_share_lock_and_release_after_exception(monkeypatch): + key = [] # Unhashable and not weak-referenceable objects must also work. + entered, release, attempted = Event(), Event(), Event() + trace: list[str] = [] + observed = ObservedLock(attempted) + # Instrument acquisition, so the first thread releases only after the second actually attempts entry. + monkeypatch.setattr(util, "RLock", lambda: ObservedLock(attempted)) + if hasattr(util, "_locks"): + monkeypatch.setattr(util, "_locks", (observed,) * util.MAX_LOCKS) + + def first_body(): + trace.append("first-enter") + entered.set() + assert release.wait(5) + trace.append("first-exit") + raise ValueError("body") + + def first(): + with pytest.raises(ValueError, match="body"): + util.lock(key, first_body) + + def second(): + assert entered.wait(5) + util.lock(key, lambda: trace.append("second-enter")) + trace.append("second-exit") + + first_thread = Thread(target=first) + first_thread.start() + assert entered.wait(5) + attempted.clear() + second_thread = Thread(target=second) + second_thread.start() + assert attempted.wait(5) + release.set() + first_thread.join(5) + second_thread.join(5) + assert not first_thread.is_alive() and not second_thread.is_alive() + assert trace == ["first-enter", "first-exit", "second-enter", "second-exit"] + assert util.lock(key, lambda: 42) == 42 + + +def test_lock_is_reentrant_and_storage_is_bounded(): + key = object() + assert util.lock(key, lambda: util.lock(key, lambda: 42)) == 42 + if hasattr(util, "_locks"): + before = util._locks + for _ in range(util.MAX_LOCKS * 2): + assert util.lock([], lambda: 1) == 1 + assert util._locks is before + assert len(util._locks) == util.MAX_LOCKS From 6383518eac77eb5fc3c2f598d4be88b5b7d921cb Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Sat, 3 Oct 2026 23:40:41 +0200 Subject: [PATCH 2/5] test(python): exercise lock through generated F# tests --- src/Fable.Build/Test/Python.fs | 4 - tests/Python/TestMisc.fs | 76 ++++++++++++++++++ tests/Python/py/lock_runtime.py | 125 ++++++++++++++++++++++++++++++ tests/Python/test_lock_runtime.py | 72 ----------------- 4 files changed, 201 insertions(+), 76 deletions(-) create mode 100644 tests/Python/py/lock_runtime.py delete mode 100644 tests/Python/test_lock_runtime.py diff --git a/src/Fable.Build/Test/Python.fs b/src/Fable.Build/Test/Python.fs index caaaffb692..87081b3596 100644 --- a/src/Fable.Build/Test/Python.fs +++ b/src/Fable.Build/Test/Python.fs @@ -23,10 +23,6 @@ let handle (args: string list) = Directory.clean buildDir - // Native runtime tests use controlled timers and real threads alongside generated F# regressions. - for testFile in Directory.GetFiles(sourceDir, "test_*.py") do - File.Copy(testFile, Path.Combine(buildDir, Path.GetFileName(testFile)), overwrite = true) - Command.Run("uv", "sync") // Install local fable-library as editable package for testing diff --git a/tests/Python/TestMisc.fs b/tests/Python/TestMisc.fs index 973873b61e..ef027191cd 100644 --- a/tests/Python/TestMisc.fs +++ b/tests/Python/TestMisc.fs @@ -1589,3 +1589,79 @@ let ``test exception variable captured in deferred single-arg closure`` () = with ex -> (fun (_: int) -> ex.Message) // lambda-with-arg captures ex sub 42 |> equal "boom" + +[] +let ``test lock is reentrant and returns the body value`` () = + let key = obj() + lock key (fun () -> lock key (fun () -> 42)) |> equal 42 + +[] +let ``test lock releases after a body exception`` () = + let key = obj() + let mutable error = "" + try + lock key (fun () -> failwith "body") + with ex -> + error <- ex.Message + equal "body" error + lock key (fun () -> 42) |> equal 42 + +#if FABLE_COMPILER +module private LockRuntime = + open Fable.Core.PyInterop + + type Threads = + abstract trace: string array + abstract errors: string array + abstract first_entered: bool + abstract second_attempted: bool + abstract second_blocked: bool + abstract threads_finished: bool + abstract record: string -> unit + abstract wait_release: unit -> bool + abstract run: Action * Action -> unit + + type Storage = + abstract capacity: int + abstract stable: bool + abstract count: int + + let makeThreads: Func = import "make_threads" "./py/lock_runtime.py" + let makeStorage: Func = import "make_storage" "./py/lock_runtime.py" + let makeKey: Func = import "make_key" "./py/lock_runtime.py" + +[] +let ``test real threads share a lock and release after an exception`` () = + let key = LockRuntime.makeKey.Invoke() + let threads = LockRuntime.makeThreads.Invoke() + let mutable error = "" + let first = Action(fun () -> + try + lock key (fun () -> + threads.record "first-enter" + threads.wait_release() |> equal true + threads.record "first-exit" + failwith "body") + with ex -> + error <- ex.Message) + let second = Action(fun () -> + lock key (fun () -> threads.record "second-enter") + threads.record "second-exit") + threads.run(first, second) + equal true threads.first_entered + equal true threads.second_attempted + equal true threads.second_blocked + equal true threads.threads_finished + equal [||] threads.errors + equal [| "first-enter"; "first-exit"; "second-enter"; "second-exit" |] threads.trace + equal "body" error + lock key (fun () -> 42) |> equal 42 + +[] +let ``test lock storage stays bounded across new objects`` () = + let storage = LockRuntime.makeStorage.Invoke() + for _ in 1 .. storage.capacity * 2 do + lock (LockRuntime.makeKey.Invoke()) (fun () -> 1) |> equal 1 + equal true storage.stable + equal storage.capacity storage.count +#endif diff --git a/tests/Python/py/lock_runtime.py b/tests/Python/py/lock_runtime.py new file mode 100644 index 0000000000..e0d0dbf86e --- /dev/null +++ b/tests/Python/py/lock_runtime.py @@ -0,0 +1,125 @@ +"""Thread and acquisition controls for the generated F# lock tests.""" + +from collections.abc import Callable +from threading import Event, RLock, Thread +from types import TracebackType +from unittest.mock import patch + +from fable_library import util +from fable_library.array_ import Array + + +class Acquisition: + def __init__(self) -> None: + self.attempted = Event() + self.blocked = False + + +class ObservedLock: + def __init__(self, acquisition: Acquisition) -> None: + self.lock = RLock() + self.acquisition = acquisition + + def __enter__(self) -> "ObservedLock": + acquired = self.lock.acquire(blocking=False) + self.acquisition.blocked = not acquired + # invariant: the contender's observation follows an actual acquisition attempt + self.acquisition.attempted.set() + if not acquired: + self.lock.acquire() + return self + + def __exit__( + self, + exc_type: type[BaseException] | None, + exc_value: BaseException | None, + traceback: TracebackType | None, + ) -> None: + self.lock.release() + + +class Threads: + def __init__(self) -> None: + self.entered = Event() + self.release = Event() + self._trace: list[str] = [] + self._errors: list[str] = [] + self.first_entered = False + self.second_attempted = False + self.second_blocked = False + self.threads_finished = False + + @property + def trace(self) -> Array[str]: + return Array(self._trace) + + @property + def errors(self) -> Array[str]: + return Array(self._errors) + + def record(self, entry: str) -> None: + self._trace.append(entry) + if entry == "first-enter": + self.entered.set() + + def wait_release(self) -> bool: + return self.release.wait(5) + + def run(self, first: Callable[[], None], second: Callable[[], None]) -> None: + acquisition = Acquisition() + stripes = tuple(ObservedLock(acquisition) for _ in range(util.MAX_LOCKS)) + + def invoke(action: Callable[[], None]) -> None: + try: + action() + except Exception as error: + self._errors.append(str(error)) + + first_thread = Thread(target=invoke, args=(first,), daemon=True) + second_thread = Thread(target=invoke, args=(second,), daemon=True) + # decision: observes both the original per-call factory and the fixed shared stripes + with ( + patch.object(util, "RLock", lambda: ObservedLock(acquisition)), + patch.object(util, "_locks", stripes, create=True), + ): + first_thread.start() + try: + self.first_entered = self.entered.wait(5) + if self.first_entered: + acquisition.attempted.clear() + second_thread.start() + self.second_attempted = acquisition.attempted.wait(5) + self.second_blocked = acquisition.blocked + finally: + self.release.set() + first_thread.join(5) + if second_thread.ident is not None: + second_thread.join(5) + self.threads_finished = not first_thread.is_alive() and not second_thread.is_alive() + + +class Storage: + def __init__(self) -> None: + self.before = getattr(util, "_locks", ()) + self.capacity = util.MAX_LOCKS + + @property + def stable(self) -> bool: + return getattr(util, "_locks", None) is self.before + + @property + def count(self) -> int: + return len(getattr(util, "_locks", ())) + + +def make_key() -> object: + # decision: uses an unhashable, non-weak-referenceable key to cover arbitrary lock objects + return [] + + +def make_threads() -> Threads: + return Threads() + + +def make_storage() -> Storage: + return Storage() diff --git a/tests/Python/test_lock_runtime.py b/tests/Python/test_lock_runtime.py deleted file mode 100644 index 37a31cc760..0000000000 --- a/tests/Python/test_lock_runtime.py +++ /dev/null @@ -1,72 +0,0 @@ -from threading import Event, RLock, Thread - -import pytest -from fable_library import util - - -class ObservedLock: - def __init__(self, observed: Event): - self.lock = RLock() - self.observed = observed - - def __enter__(self): - acquired = self.lock.acquire(blocking=False) - self.observed.set() - if not acquired: - self.lock.acquire() - return self - - def __exit__(self, *args): - self.lock.release() - - -def test_real_threads_share_lock_and_release_after_exception(monkeypatch): - key = [] # Unhashable and not weak-referenceable objects must also work. - entered, release, attempted = Event(), Event(), Event() - trace: list[str] = [] - observed = ObservedLock(attempted) - # Instrument acquisition, so the first thread releases only after the second actually attempts entry. - monkeypatch.setattr(util, "RLock", lambda: ObservedLock(attempted)) - if hasattr(util, "_locks"): - monkeypatch.setattr(util, "_locks", (observed,) * util.MAX_LOCKS) - - def first_body(): - trace.append("first-enter") - entered.set() - assert release.wait(5) - trace.append("first-exit") - raise ValueError("body") - - def first(): - with pytest.raises(ValueError, match="body"): - util.lock(key, first_body) - - def second(): - assert entered.wait(5) - util.lock(key, lambda: trace.append("second-enter")) - trace.append("second-exit") - - first_thread = Thread(target=first) - first_thread.start() - assert entered.wait(5) - attempted.clear() - second_thread = Thread(target=second) - second_thread.start() - assert attempted.wait(5) - release.set() - first_thread.join(5) - second_thread.join(5) - assert not first_thread.is_alive() and not second_thread.is_alive() - assert trace == ["first-enter", "first-exit", "second-enter", "second-exit"] - assert util.lock(key, lambda: 42) == 42 - - -def test_lock_is_reentrant_and_storage_is_bounded(): - key = object() - assert util.lock(key, lambda: util.lock(key, lambda: 42)) == 42 - if hasattr(util, "_locks"): - before = util._locks - for _ in range(util.MAX_LOCKS * 2): - assert util.lock([], lambda: 1) == 1 - assert util._locks is before - assert len(util._locks) == util.MAX_LOCKS From 1ba1dcd9d379a18486e63a6f49c67947219aaedd Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Sun, 4 Oct 2026 00:14:07 +0200 Subject: [PATCH 3/5] fix(python): await StartChild timeout regressions --- src/Fable.Transforms/Python/Replacements.fs | 3 ++ src/fable-library-py/fable_library/async_.py | 43 +++++++++++++------ .../tests/test_async_timeout.py | 25 +++++++++++ tests/Python/TestAsync.fs | 8 ++-- 4 files changed, 61 insertions(+), 18 deletions(-) create mode 100644 src/fable-library-py/tests/test_async_timeout.py diff --git a/src/Fable.Transforms/Python/Replacements.fs b/src/Fable.Transforms/Python/Replacements.fs index 2f18e27862..d19de63b55 100644 --- a/src/Fable.Transforms/Python/Replacements.fs +++ b/src/Fable.Transforms/Python/Replacements.fs @@ -3672,6 +3672,9 @@ let asyncBuilder (com: ICompiler) (ctx: Context) r t (i: CallInfo) (thisArg: Exp let asyncs com (ctx: Context) r t (i: CallInfo) (_: Expr option) (args: Expr list) = match i.CompiledName with + | "StartChild" -> + Helper.LibCall(com, "async_", "start_child_with_timeout", t, args, i.SignatureArgTypes, ?loc = r) + |> Some | "Start" -> Helper.LibCall(com, "async_", "start", t, args, i.SignatureArgTypes, ?loc = r) |> Some diff --git a/src/fable-library-py/fable_library/async_.py b/src/fable-library-py/fable_library/async_.py index 1b72efd6ee..36c814ac79 100644 --- a/src/fable-library-py/fable_library/async_.py +++ b/src/fable-library-py/fable_library/async_.py @@ -32,6 +32,7 @@ ) from .event import IEvent_2 from .protocols import IEnumerable_1 +from .system import TimeoutException__ctor from .task import TaskCompletionSource from .time_span import TimeSpan, to_milliseconds from .types import UNIT, Unit @@ -291,23 +292,36 @@ def cancel(_: OperationCanceledError) -> None: return tcs.get_task() -def start_child[T](computation: Async[T], ms: int | TimeSpan | None = None) -> Async[Async[T]]: - if ms is not None: - # Race the computation against a timeout: whichever settles first wins. - # asyncio.gather (the previous implementation via parallel2) waited for BOTH to settle, - # which meant the timeout always fired even when the computation finished first. - task = start_as_task(computation) +def _start_child_timeout[T]( + computation: Async[T], ms: int | TimeSpan, timeout_error: Callable[[], Exception] +) -> Async[Async[T]]: + # Race the computation against a timeout: whichever settles first wins. + # asyncio.gather (the previous implementation via parallel2) waited for BOTH to settle, + # which meant the timeout always fired even when the computation finished first. + task = start_as_task(computation) + + async def with_timeout() -> T: + try: + return await asyncio.wait_for(task, timeout=to_milliseconds(ms) / 1000.0) + except TimeoutError: + raise timeout_error() + + def cont(ctx: IAsyncContext[Async[T]]) -> None: + protected_return(await_task(with_timeout()))(ctx) + + return protected_cont(cont) - async def with_timeout() -> T: - try: - return await asyncio.wait_for(task, timeout=to_milliseconds(ms) / 1000.0) - except TimeoutError: - raise TimeoutError() - def cont(ctx: IAsyncContext[Async[T]]) -> None: - protected_return(await_task(with_timeout()))(ctx) +def start_child_with_timeout[T](computation: Async[T], ms: int | TimeSpan | None = None) -> Async[Async[T]]: + # decision: use a new entry point so existing Python callers retain the built-in TimeoutError contract + if ms is None: + return start_child(computation) + return _start_child_timeout(computation, ms, TimeoutException__ctor) - return protected_cont(cont) + +def start_child[T](computation: Async[T], ms: int | TimeSpan | None = None) -> Async[Async[T]]: + if ms is not None: + return _start_child_timeout(computation, ms, TimeoutError) task = start_as_task(computation) @@ -401,6 +415,7 @@ def exception_handler(loop: asyncio.AbstractEventLoop, context: dict[str, Any]) "start", "start_as_task", "start_child", + "start_child_with_timeout", "start_immediate", "start_with_continuations", ] diff --git a/src/fable-library-py/tests/test_async_timeout.py b/src/fable-library-py/tests/test_async_timeout.py new file mode 100644 index 0000000000..556eea68c2 --- /dev/null +++ b/src/fable-library-py/tests/test_async_timeout.py @@ -0,0 +1,25 @@ +import asyncio +from collections.abc import Callable + +import pytest +from fable_library.async_ import start_as_task, start_child, start_child_with_timeout +from fable_library.async_builder import Async, IAsyncContext +from fable_library.system import TimeoutException + + +@pytest.mark.parametrize( + ("start_child_fn", "timeout_type"), + [(start_child, TimeoutError), (start_child_with_timeout, TimeoutException)], +) +def test_start_child_timeout_preserves_exception_contract( + start_child_fn: Callable[[Async[None], int], Async[Async[None]]], timeout_type: type[Exception] +) -> None: + def pending(_ctx: IAsyncContext[None]) -> None: + pass + + async def run() -> None: + child = await start_as_task(start_child_fn(pending, 0)) + await start_as_task(child) + + with pytest.raises(timeout_type): + asyncio.run(run()) diff --git a/tests/Python/TestAsync.fs b/tests/Python/TestAsync.fs index 6db0fdf8c3..0b6e80a8e5 100644 --- a/tests/Python/TestAsync.fs +++ b/tests/Python/TestAsync.fs @@ -467,7 +467,7 @@ let ``test Async.StartChild applies timeout`` () = let mutable x = "" let task = async { - x <- x + "A" + // A busy .NET runner can time out before this child is scheduled. do! Async.Sleep 1_000 x <- x + "X" // Never hit } @@ -482,8 +482,8 @@ let ``test Async.StartChild applies timeout`` () = x <- x + "C" - equal x "ABC" - } |> Async.StartImmediate + equal "BC" x + } |> Async.RunSynchronously [] let ``test Async.StartChild with timeout completes when computation finishes before timeout`` () = // See #4481 @@ -496,7 +496,7 @@ let ``test Async.StartChild with timeout completes when computation finishes bef with | :? TimeoutException -> failwith "should not time out" - } |> Async.StartImmediate + } |> Async.RunSynchronously [] let ``test Unit arguments are erased`` () = // See #1832 From 1fd0b5ff28494314c835049b4063eae94f0ad36d Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Sun, 4 Oct 2026 00:26:13 +0200 Subject: [PATCH 4/5] test(python): run timeout checks with generated runtime --- src/Fable.Build/Test/Python.fs | 4 ++++ .../tests => tests/Python}/test_async_timeout.py | 0 2 files changed, 4 insertions(+) rename {src/fable-library-py/tests => tests/Python}/test_async_timeout.py (100%) diff --git a/src/Fable.Build/Test/Python.fs b/src/Fable.Build/Test/Python.fs index 87081b3596..ea0cde852e 100644 --- a/src/Fable.Build/Test/Python.fs +++ b/src/Fable.Build/Test/Python.fs @@ -23,6 +23,10 @@ let handle (args: string list) = Directory.clean buildDir + // These tests depend on F#-generated runtime modules unavailable in the source-only package. + for testFile in Directory.GetFiles(sourceDir, "test_*.py") do + File.Copy(testFile, Path.Combine(buildDir, Path.GetFileName(testFile)), overwrite = true) + Command.Run("uv", "sync") // Install local fable-library as editable package for testing diff --git a/src/fable-library-py/tests/test_async_timeout.py b/tests/Python/test_async_timeout.py similarity index 100% rename from src/fable-library-py/tests/test_async_timeout.py rename to tests/Python/test_async_timeout.py From c065be34949604d3dbedc5a6141e90406a9ce814 Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Sun, 4 Oct 2026 10:52:37 +0200 Subject: [PATCH 5/5] fix(python): use weak per-object locks instead of stripes (#5039) --- src/fable-library-py/fable_library/async_.py | 4 +- src/fable-library-py/fable_library/util.py | 20 ++++++---- tests/Python/TestMisc.fs | 40 ++++++++++++++++++-- tests/Python/py/lock_runtime.py | 24 ++++++++++-- 4 files changed, 70 insertions(+), 18 deletions(-) diff --git a/src/fable-library-py/fable_library/async_.py b/src/fable-library-py/fable_library/async_.py index 36c814ac79..3035b42909 100644 --- a/src/fable-library-py/fable_library/async_.py +++ b/src/fable-library-py/fable_library/async_.py @@ -295,9 +295,7 @@ def cancel(_: OperationCanceledError) -> None: def _start_child_timeout[T]( computation: Async[T], ms: int | TimeSpan, timeout_error: Callable[[], Exception] ) -> Async[Async[T]]: - # Race the computation against a timeout: whichever settles first wins. - # asyncio.gather (the previous implementation via parallel2) waited for BOTH to settle, - # which meant the timeout always fired even when the computation finished first. + # Return the child result when ready, or raise the selected timeout error if it takes too long. task = start_as_task(computation) async def with_timeout() -> T: diff --git a/src/fable-library-py/fable_library/util.py b/src/fable-library-py/fable_library/util.py index d6797f14f7..1d3f5648ed 100644 --- a/src/fable-library-py/fable_library/util.py +++ b/src/fable-library-py/fable_library/util.py @@ -29,6 +29,7 @@ overload, ) from urllib.parse import quote, unquote +from weakref import WeakValueDictionary from .array_ import Array from .bases import ( @@ -351,16 +352,21 @@ def assert_not_equal[T](actual: T, expected: T, msg: str | None = None) -> None: raise Exception(msg or f"Expected not equal to: {expected} - Actual: {actual}") -MAX_LOCKS = 1024 +# decision: weak values give each active object its own lock without retaining idle locks or their objects +_locks: WeakValueDictionary[int, RLock] = WeakValueDictionary() +_lock_registry_guard = RLock() -# decision: fixed stripes bound synchronization storage without evicting a lock held by another thread -# tradeoff: unrelated objects can share a stripe and serialize, but object identity always selects the same lock -_locks = tuple(RLock() for _ in range(MAX_LOCKS)) +def lock[T](lock_obj: object, fn: Callable[[], T]) -> T: + key = id(lock_obj) + with _lock_registry_guard: + object_lock = _locks.get(key) + if object_lock is None: + object_lock = RLock() + _locks[key] = object_lock - -def lock[T](lock_obj: Any, fn: Callable[[], T]) -> T: - with _locks[(id(lock_obj) >> 4) % MAX_LOCKS]: + # invariant: each holder and waiter keeps a strong lock reference outside the registry guard + with object_lock: return fn() diff --git a/tests/Python/TestMisc.fs b/tests/Python/TestMisc.fs index ef027191cd..769d89fc7a 100644 --- a/tests/Python/TestMisc.fs +++ b/tests/Python/TestMisc.fs @@ -1622,13 +1622,13 @@ module private LockRuntime = abstract run: Action * Action -> unit type Storage = - abstract capacity: int abstract stable: bool abstract count: int let makeThreads: Func = import "make_threads" "./py/lock_runtime.py" let makeStorage: Func = import "make_storage" "./py/lock_runtime.py" let makeKey: Func = import "make_key" "./py/lock_runtime.py" + let makeCollidingKeys: Func = import "make_colliding_keys" "./py/lock_runtime.py" [] let ``test real threads share a lock and release after an exception`` () = @@ -1658,10 +1658,42 @@ let ``test real threads share a lock and release after an exception`` () = lock key (fun () -> 42) |> equal 42 [] -let ``test lock storage stays bounded across new objects`` () = +let ``test independent objects do not share a lock`` () = + let keys = LockRuntime.makeCollidingKeys.Invoke() + let threads = LockRuntime.makeThreads.Invoke() + let first = Action(fun () -> + lock keys.[0] (fun () -> + threads.record "first-enter" + threads.wait_release() |> equal true + threads.record "first-exit")) + let second = Action(fun () -> + lock keys.[1] (fun () -> threads.record "second-enter") + threads.record "second-exit") + threads.run(first, second) + equal true threads.first_entered + equal true threads.second_attempted + equal false threads.second_blocked + equal true threads.threads_finished + equal [||] threads.errors + equal 4 threads.trace.Length + +[] +let ``test lock storage releases idle entries across new objects`` () = + let iterations = 2048 let storage = LockRuntime.makeStorage.Invoke() - for _ in 1 .. storage.capacity * 2 do + let before = storage.count + lock (LockRuntime.makeKey.Invoke()) (fun () -> equal (before + 1) storage.count) + for _ in 1 .. iterations do lock (LockRuntime.makeKey.Invoke()) (fun () -> 1) |> equal 1 equal true storage.stable - equal storage.capacity storage.count + equal before storage.count + +[] +let ``test lock storage releases idle entries after exceptions`` () = + let storage = LockRuntime.makeStorage.Invoke() + let before = storage.count + try + lock (LockRuntime.makeKey.Invoke()) (fun () -> failwith "body") + with _ -> () + equal before storage.count #endif diff --git a/tests/Python/py/lock_runtime.py b/tests/Python/py/lock_runtime.py index e0d0dbf86e..22f29ffd71 100644 --- a/tests/Python/py/lock_runtime.py +++ b/tests/Python/py/lock_runtime.py @@ -1,14 +1,19 @@ """Thread and acquisition controls for the generated F# lock tests.""" +import gc from collections.abc import Callable from threading import Event, RLock, Thread from types import TracebackType from unittest.mock import patch +from weakref import WeakValueDictionary from fable_library import util from fable_library.array_ import Array +LEGACY_STRIPE_COUNT = 1024 + + class Acquisition: def __init__(self) -> None: self.attempted = Event() @@ -67,7 +72,6 @@ def wait_release(self) -> bool: def run(self, first: Callable[[], None], second: Callable[[], None]) -> None: acquisition = Acquisition() - stripes = tuple(ObservedLock(acquisition) for _ in range(util.MAX_LOCKS)) def invoke(action: Callable[[], None]) -> None: try: @@ -77,10 +81,9 @@ def invoke(action: Callable[[], None]) -> None: first_thread = Thread(target=invoke, args=(first,), daemon=True) second_thread = Thread(target=invoke, args=(second,), daemon=True) - # decision: observes both the original per-call factory and the fixed shared stripes with ( patch.object(util, "RLock", lambda: ObservedLock(acquisition)), - patch.object(util, "_locks", stripes, create=True), + patch.object(util, "_locks", WeakValueDictionary()), ): first_thread.start() try: @@ -101,7 +104,6 @@ def invoke(action: Callable[[], None]) -> None: class Storage: def __init__(self) -> None: self.before = getattr(util, "_locks", ()) - self.capacity = util.MAX_LOCKS @property def stable(self) -> bool: @@ -109,6 +111,7 @@ def stable(self) -> bool: @property def count(self) -> int: + gc.collect() return len(getattr(util, "_locks", ())) @@ -117,6 +120,19 @@ def make_key() -> object: return [] +def make_colliding_keys() -> Array[object]: + # decision: reproduces the former stripe collision using distinct, unhashable objects + keys: dict[int, object] = {} + for _ in range(LEGACY_STRIPE_COUNT + 1): + key: object = [] + stripe = (id(key) >> 4) % LEGACY_STRIPE_COUNT + previous = keys.get(stripe) + if previous is not None: + return Array([previous, key]) + keys[stripe] = key + raise AssertionError("Expected a collision in the former fixed stripes") + + def make_threads() -> Threads: return Threads()