Skip to content

Prevent LocalDiscovery teardown from crashing on shared-memory double-unlink — Closes #291#302

Merged
conradbzura merged 4 commits into
wool-labs:releasefrom
conradbzura:291-fix-local-discovery-teardown
Jul 14, 2026
Merged

Prevent LocalDiscovery teardown from crashing on shared-memory double-unlink — Closes #291#302
conradbzura merged 4 commits into
wool-labs:releasefrom
conradbzura:291-fix-local-discovery-teardown

Conversation

@conradbzura

@conradbzura conradbzura commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Prevent LocalDiscovery teardown from crashing the interpreter when a shared-memory segment is unlinked twice.

The owner's __exit__ unlinked its segment unguarded, so a segment already reclaimed by another process's resource tracker raised FileNotFoundError out of teardown — and because atexit.unregister ran after that raising unlink, the shutdown fallback stayed armed and fired a second unlink at interpreter exit. Two bugs compounding: the first crashed the caller's teardown, the second crashed the interpreter.

Fix the ordering (disarm the fallback before unlinking, so a failed unlink can never leave it armed) and route every unlink in the module through a single _unlink_quietly helper. The module previously carried four "unlink and tolerate failure" sites under two different policies — the address-space paths suppressed only FileNotFoundError while the block paths swallowed every OSError, justified by a Windows rationale that does not hold (SharedMemory.unlink is a documented no-op on Windows). The real reason suppression is needed is narrower and verifiable: any process that attached to a segment may have unlinked it first (bpo-38119), which surfaces as ENOENT. _unlink_quietly therefore swallows FileNotFoundError silently and downgrades every other OSError to a ResourceWarning rather than raising it. Teardown can no longer replace an exception the caller is already unwinding, and a genuine leak stays observable instead of being silently swallowed.

Two adjacent reclamation paths that could not fire are fixed alongside it. Publisher._add delegated failure cleanup to _drop, which scans the address space for a ref that _add only writes on the last line of its try — so on any failure the ref was never there, _drop found nothing, and the block leaked with its atexit handler armed for the pool's lifetime. And re-entering one LocalDiscovery instance silently demoted it to non-owner while stranding the first entry's fallback; instances are now single-use.

Trade-offs

LocalDiscovery instances are now single-use. A second __enter__ raises RuntimeError whether or not the first has exited, so sequentially reusing an instance — which worked before — now fails. Nothing in the codebase relied on it, every internal caller already constructs a fresh instance per with, and failing loudly is the point: the silent alternative corrupted ownership state and leaked the namespace.

An unexpected unlink failure now warns instead of raising. A non-ENOENT OSError during teardown emits a ResourceWarning rather than propagating. This is deliberate — a raising teardown replaces the caller's in-flight exception — but it means a genuinely unreclaimable segment no longer stops the program. ResourceWarning is in Python's default ignore filter, so it is silent in a normal run and visible under -X dev, -W, or pytest, matching how CPython treats an unclosed socket.

Two known bugs are pinned as strict xfails rather than fixed here. capacity is not enforced (POSIX page-rounding inflates the segment to a full page regardless, #299), and a surviving pool's worker leaks after an owner-first exit (#298). Both tests flip to XPASS — and, being strict, to hard failures — the moment those issues land, forcing the markers to be removed.

Closes #291

Proposed changes

Order teardown so a failed unlink cannot leave the fallback armed

__exit__ now unregisters the atexit fallback before closing and unlinking. Previously the unregister sat after the unlink, so any unlink failure skipped it and left the handler armed to fire a second time at shutdown — the crash #291 reports.

Route every unlink through one policy

Add the module-private _unlink_quietly(shared_memory) and call it from all four teardown sites: __enter__'s fallback closure, __exit__, Publisher._shared_memory_factory's fallback closure, and Publisher._shared_memory_finalizer. It swallows FileNotFoundError — the segment is already gone, which is the expected outcome under bpo-38119 — and emits a ResourceWarning for any other OSError.

This replaces the two divergent policies with one, and it is what makes teardown non-raising: an exception escaping __exit__ or an atexit handler would otherwise replace the exception the caller was unwinding, or crash the interpreter at shutdown. Swallowing everything would have traded that bug for a silent leak, so an unexpected failure warns instead.

Release the block a failed publish acquired

Publisher._add acquires a per-worker block, packs the metadata into it, and only then writes the worker's ref into the address space. On any failure it called _drop, which reclaims a block by scanning the address space for that ref — but the ref lands on the last line of the try, so on every failure path it was never written. _drop could not find it, release never ran, and the block survived with its atexit handler armed until the publisher closed. Worse, the leak was sticky: a later successful add of the same uid took the refcount to 2, so its matching drop returned it to 1 and the block was never unlinked even on a clean drop.

Release what _add itself acquired instead of inferring it from address-space state. ResourcePool.release is a documented no-op for a key that was never cached, so it is also correct when acquire is what raised.

Guard LocalDiscovery against re-entry

Entering the same instance twice overwrote _address_space and _owner while leaving _cleanup pointing at the first entry's closure, so both exits took the non-owner branch, the fallback was never disarmed, and the namespace stayed occupied for the process lifetime. Decorate __enter__ with the existing @noreentry utility so a second entry raises RuntimeError instead of silently corrupting the instance's ownership state. The guard binds to the instance, not the namespace — distinct instances sharing a namespace compare equal but guard independently.

Document the teardown contract on the public surface

LocalDiscovery is public but carried no statement of the behavior this change is built around. Its class docstring now states the contract once: create-or-attach ownership, the owner's exit unlinking the segment for every still-attached peer, the shutdown fallback for a context never exited, the single-use guard, and a pointer to _unlink_quietly for the failure semantics. The Publisher/Subscriber examples were also corrected — as written they raised FileNotFoundError, because they showed no owning context.

Test cases

# Test Suite Given When Then Coverage Target
1 TestLocalDiscovery An owner and a non-owner sharing a namespace Both enter and exit Workers are preserved for the peer that joined an existing namespace Create-or-attach ownership
2 TestLocalDiscovery A namespace entered and exited three times A fresh instance enters again The segment is recreated, proven by a publish raising FileNotFoundError between cycles Segment name is freed on teardown
3 TestLocalDiscovery An owner already holding a namespace A second LocalDiscovery enters and exits the same namespace No atexit fallback is registered or unregistered Non-owner performs no fallback traffic
4 TestLocalDiscovery A LocalDiscovery instance already entered The same instance is entered again It raises RuntimeError Single-use guard
5 TestLocalDiscovery Two instances that compare and hash equal by namespace Each is entered in turn Both enter successfully Guard binds to the instance, not the namespace
6 TestLocalDiscovery An instance whose second entry was rejected The instance exits the block it did enter Every registered fallback is unregistered Rejected re-entry strands nothing
7 TestLocalDiscovery An owner holding a namespace The owner exits The segment is unlinked, proven by a subsequent publish raising FileNotFoundError Owner exit unlinks for all peers
8 TestLocalDiscovery An owner with a clean exit The owner exits The atexit fallback is disarmed Fallback pairing
9 TestLocalDiscovery An owner whose body raises The owner exits by exception The segment is still unlinked Teardown runs on the exceptional path
10 TestLocalDiscovery An owner and a non-owner with overlapping lifetimes The owner exits before the non-owner Both unwind cleanly and the namespace is re-creatable Owner-first unwind
11 TestLocalDiscovery Three interleaved discovery lifecycles The contexts unwind in overlapping order A publish through the last epoch is discovered by a subscriber Interleaved lifecycles
12 TestLocalDiscovery An owner whose segment vanished before exit The owner exits It exits cleanly without raising ENOENT is the expected case
13 TestLocalDiscovery An owner whose unlink raises FileNotFoundError The owner exits The fallback is unregistered Disarm precedes unlink
14 TestLocalDiscovery An owner whose unlink raises PermissionError The owner exits The fallback is still unregistered Disarm precedes unlink on unexpected failure
15 TestLocalDiscovery An owner whose unlink raises PermissionError The owner exits It emits a ResourceWarning naming the segment Unexpected failure is observable
16 TestLocalDiscovery An owner whose unlink raises PermissionError The body raises ValueError and the owner exits The body's ValueError propagates, not the teardown failure Teardown never masks the caller's exception
17 TestLocalDiscovery Any tree of nested and sequential lifecycles (Hypothesis) The tree is entered and unwound Every registered fallback is unregistered Fallback pairing under arbitrary interleavings
18 TestLocalDiscovery Any tree of lifecycles with segments vanishing at arbitrary points (Hypothesis) The tree is entered and unwound It unwinds cleanly and every fallback is paired Pairing survives vanished segments
19 TestLocalDiscoveryPublisher A publisher and an unknown worker publish("worker-dropped", …) is called It completes silently and emits no event Dropping an unknown worker is a no-op
20 TestLocalDiscoveryPublisher A published worker The worker is dropped The block's atexit fallback is disarmed Block reclamation on drop
21 TestLocalDiscoveryPublisher A worker added and dropped repeatedly The cycle repeats Fallback registrations and unregistrations stay paired Block pairing across cycles
22 TestLocalDiscoveryPublisher A published worker whose block was unlinked externally The worker is dropped The drop completes and the fallback is still paired Drop tolerates a vanished block
23 TestLocalDiscoveryPublisher A published worker whose block unlink raises RuntimeError The worker is dropped The fallback was already disarmed Disarm precedes unlink for blocks
24 TestLocalDiscoveryPublisher A publisher and a worker whose metadata exceeds the block The oversized worker is published and the add fails The acquired block is released and its fallback paired Failed add reclaims its block
25 TestLocalDiscoveryPublisher An oversized worker and a fitting worker Both are published Only the fitting worker is discoverable Failed add leaves no trace
26 TestLocalDiscoveryPublisher A publisher with workers still published The publisher exits Every block fallback is disarmed Publisher teardown reclaims blocks
27 TestLocalDiscoveryPublisher A publisher whose worker segments were already unlinked The publisher exits It exits cleanly Teardown tolerates vanished blocks
28 TestLocalDiscoveryPublisher Any sequence of adds and drops (Hypothesis) The sequence is applied Fallback registrations and unregistrations stay paired Block pairing under arbitrary sequences
29 TestLocalDiscoveryPublisher A discovery with capacity=8 Nine workers are published The ninth raises RuntimeError Address-space exhaustion (xfail(strict) pending #299)
30 TestSameNamespaceRespawn A worker pool repeatedly created and torn down on one namespace The pools respawn rapidly Each unwinds cleanly and every worker is reaped Rapid namespace respawn
31 TestOverlappingNamespaceLifecycles Two pools sharing a namespace The non-owner pool exits first Both unwind cleanly Non-owner-first teardown
32 TestOverlappingNamespaceLifecycles Two pools sharing a namespace The owner pool exits first Both unwind cleanly with no leaked worker Owner-first teardown
33 TestOverlappingNamespaceLifecycles Two pools sharing a namespace The owner pool exits first The surviving pool's worker is reaped Worker reaping (xfail(strict) pending #298)
34 TestCrossProcessTeardown An owner process whose segment is unlinked by another process's resource tracker The owner exits It exits with returncode 0 and no traceback Real double-unlink across processes
35 TestCrossProcessTeardown An owner process that never exits its context The interpreter shuts down The armed fallback reclaims the segment without crashing Fallback survives real interpreter shutdown

@conradbzura conradbzura self-assigned this Jul 13, 2026
Unlinking a shared-memory segment that another process had already
removed raised FileNotFoundError out of the owner's context exit,
aborting pool teardown, and the raise skipped the atexit unregister so
the fallback fired a second FileNotFoundError at interpreter shutdown.
The failure is reliable under rapid same-namespace pool teardown and
respawn, where a dying process's multiprocessing resource tracker
unlinks the deterministically named segment out from under the owner.

Owner exit now disarms the atexit fallback before unlinking and treats
a missing segment as a no-op, the fallback closure suppresses the same
error at shutdown, and the publisher block finalizer applies the same
disarm-before-unlink ordering so a failed unlink can never leave a
fallback armed.
Add regression tests for the double-unlink crash and the atexit
fallback double-fire, then pin the surrounding teardown contracts:
owner exit unlinks the segment, exceptional exit still tears down,
non-owners neither arm fallbacks nor unlink, namespaces stay reusable
across rapid enter/exit cycles, and overlapping same-namespace
generations unwind cleanly. Recording atexit wrappers pin the
disarm-before-unlink ordering for both the owner exit and the
publisher block finalizer, where an unsuppressed unlink error is the
only observable seam. Hypothesis properties generalize the regression
cases across arbitrary lifecycle interleavings, vanishing-segment
masks, and publisher add/drop sequences. Reword two overclaiming test
docstrings to match what their bodies assert.
Exercise the reported reproduction shapes end to end with real worker
pools: rapid same-namespace teardown and respawn, LIFO overlap, and
owner-first exit with a post-overlap respawn. A cross-process test
proves the genuine failure mechanism by letting an independent
interpreter's resource tracker unlink the shared segment and asserting
the owner still exits cleanly through real interpreter shutdown. A
strict xfail pins the known surviving-pool worker leak so the suite
flags its fix loudly when it lands.
A leaked-context owner interpreter shuts down with the atexit fallback
still armed after an independent attacher's resource tracker unlinked
the segment. The pre-fix bare lambda leaves an atexit traceback on
stderr at shutdown; the suppressing closure shuts down silently. This
is the first behavioral pin for the fallback's own suppression, which
coverage cannot see because it runs in an unmeasured interpreter.
@conradbzura conradbzura force-pushed the 291-fix-local-discovery-teardown branch from 2dca9a2 to c4c6faf Compare July 13, 2026 22:18
@conradbzura conradbzura marked this pull request as ready for review July 14, 2026 00:36
@conradbzura conradbzura merged commit 79ce720 into wool-labs:release Jul 14, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prevent LocalDiscovery teardown from crashing on shared-memory double-unlink

1 participant