Extension-table reclaim-on-owner-exit reintroduces the #2881 failure mode that #3014 fixed (tensor-pool); reachability API left as dead code

[phil-opp]

Summary

Two PRs that merged within hours of each other on 2026-08-13 pull in opposite directions on memory-pool lifetime, and the later one silently drops the guarantee the earlier one established:

• fix(daemon): reclaim memory pools once no live node can reach them #3014 (1ccbc00, "fix(daemon): reclaim memory pools once no live node can reach them") fixed Memory pools may not be released when a node crashes or is dynamically removed #2881 by making pool reclamation reachability-based: an entry records potential_readers (the registrar's transitive downstream closure) plus touched_by, and is only released once no live node can reach it. Its commit message is explicit that the naive alternative is wrong:

  Eagerly freeing a node's pools when it goes away is not correct […] a pool deliberately outlives its registrar — the normal lifecycle is that a sender registers a pool and a receiver reads and frees it, possibly well after the sender exited.

• refactor(tensor-pool): move behind a generic extension seam, opt-in and outside the 1.0 guarantees #3152 (0442564, "refactor(tensor-pool): move behind a generic extension seam") then replaced the daemon's pool table with the new generic ExtensionTable, which reclaims on owner (storing-node) exit — i.e. exactly the eager-free behavior fix(daemon): reclaim memory pools once no live node can reach them #3014 declared incorrect.

The net effect: for the reinstated (opt-in) tensor-pool extension, the #2881 fix is no longer in force, and the code that actually implemented it is now dead.

Evidence

1. The daemon reclaims a descriptor the moment its storing node exits — clean exit included.

binaries/daemon/src/lib.rs:5883 calls reclaim_extensions_of_exited_node on every node-result path (crash, clean exit, restart), and that calls ExtensionTable::reclaim_owner (binaries/daemon/src/extension_table.rs:127), which drops every entry owned by the exiting node unconditionally. There is no potential_readers/reachability concept anywhere in ExtensionTable. The in-code comment only justifies the crash case:

// A node that crashed cannot withdraw its own descriptors.
// Reclaiming here is the reason the extension table lives in the daemon (dora-rs/dora#2881).

…but the same path fires on a clean exit of a bounded producer that has already delivered its last tensor.

2. The notification does not cover the in-flight receiver.

docs/extensions.md:44 says every node that "stored or read" a key is notified on drop. A receiver that has been handed the buffer_id via a normal dataflow message but has not yet called extension_load is not in touched_by, so it is not notified — it simply finds the descriptor gone on its next read. That is precisely the #2881 shape ("the receiver keeps a mapping to memory nobody owns, and nothing ever tells it otherwise"), which docs/extensions.md:74-81 claims this seam solves.

3. The read path depends on the descriptor surviving the sender.

libraries/extensions/tensor-pool/python/src/seam.rs:68 (load_metadata) turns a missing descriptor into a hard error ("tensor pool {buffer_id} has no descriptor"). In read_tensor_pool (libraries/extensions/tensor-pool/python/src/transport.rs:2345) this feeds the trusted GPU buffer-size validation (first read) and the slow/fallback read path (transport.rs:2405). So a receiver reading a pool after the registering sender has cleanly exited loses GPU size validation and the fallback path — the exact scenario #3014's potential_readers was built to keep working.

4. The #3014 fix is now dead code.

dora-tensor-pool's TensorPoolManager still carries the whole reachability API — register_tensor_pool(…, potential_readers), extend_potential_readers, reclaim_unreachable, cleanup_dataflow (libraries/extensions/tensor-pool/src/lib.rs:120,288,309,330) — but it is never constructed outside its own test module. Workspace-wide, the only thing the daemon calls from that crate is the free function cleanup_orphans (binaries/daemon/src/lib.rs:3832):

$ grep -rn "TensorPoolManager::new\|reclaim_unreachable\|extend_potential_readers" --include=*.rs .   # (excluding the crate's own file)
    → (no matches)

Because every method is pub, -D warnings does not flag it, which is why CI stayed green. The reachability logic was faithfully carried into the parked crate but disconnected from the daemon, and the replacement (ExtensionTable) does not re-implement it.

Risk / impact

• Correctness: a bounded/one-shot sender that registers a tensor pool, sends the id, and exits can have its descriptor reclaimed before a downstream consumer loads it — breaking the transfer (GPU size validation + slow-path read). This is the Memory pools may not be released when a node crashes or is dynamically removed #2881 failure mode, reintroduced.
• Scope is bounded: the tensor-pool extension is opt-in (--features tensor-pool), off by default, and explicitly outside the 1.0 compatibility guarantees, with no in-tree e2e coverage — so default builds are unaffected. The fast DORADMA CPU path reconstructs from the shmem segment by name and may still succeed while the segment exists, so the manifestation is timing- and path-dependent rather than a guaranteed hard failure. That is why this is filed as a latent regression / semantic gap rather than a crash-every-time bug.
• Maintainability: ~600 lines of reachability manager + tests in dora-tensor-pool are dead, giving a misleading impression that the Memory pools may not be released when a node crashes or is dynamically removed #2881 fix travels with the extension.

Suggested direction

Either (a) teach the generic ExtensionTable a reachability notion so a descriptor survives its owner's clean exit until no potential downstream reader remains (mirroring #3014's potential_readers/extend_potential_readers/reclaim_unreachable, which the daemon would need to drive from the running dataflow's downstream closure), or (b) if owner-exit reclamation is the intended, accepted semantics for the extension seam, wire the surviving TensorPoolManager back in (or delete it) and update docs/extensions.md to stop claiming the seam resolves #2881 — since for a clean producer exit it does not.

---

This issue was created by a scheduled automated Claude code-review check (reviewing the last 2 days of merged PRs/commits). It reflects static analysis of the interaction between #3014 and #3152; the runtime manifestation depends on tensor-pool read-path timing and has not been reproduced live. Please verify before acting.

[phil-opp]

I verified the two load-bearing claims against current main and they hold:

1. ExtensionTable::reclaim_owner reclaims on owner exit unconditionally. binaries/daemon/src/extension_table.rs filters purely on k.dataflow_id == dataflow_id && e.owner == node and drops every match — there is no potential_readers/reachability notion anywhere in the table. The reclaim_extensions_of_exited_node caller (binaries/daemon/src/lib.rs) fires on every node-result path, clean exit included.
2. The fix(daemon): reclaim memory pools once no live node can reach them #3014 reachability API is dead outside its own crate. grep -rn "TensorPoolManager::new\|reclaim_unreachable\|extend_potential_readers" --include=*.rs . returns no matches outside libraries/extensions/tensor-pool/src/lib.rs itself. The daemon only calls the free function cleanup_orphans.

So the interaction the issue describes is real: for the opt-in tensor-pool extension, #3152's ExtensionTable reclaims a descriptor on the storing node's clean exit, which is the eager-free behavior #3014 deliberately replaced with reachability-based reclamation, and the reachability code that implemented #3014 is now unreachable.

I'm not opening a fix PR because the resolution is a design decision that needs a maintainer, not a mechanical change:

• (a) Teach ExtensionTable a reachability notion so a descriptor survives its owner's clean exit until no potential downstream reader remains — the daemon would need to drive the downstream closure from the running dataflow, re-deriving fix(daemon): reclaim memory pools once no live node can reach them #3014's potential_readers/extend_potential_readers/reclaim_unreachable on the generic seam.
• (b) Accept owner-exit reclamation as the intended semantics for the extension seam, then wire the surviving TensorPoolManager back in (or delete it) and update docs/extensions.md so it no longer claims the seam resolves Memory pools may not be released when a node crashes or is dynamically removed #2881 — since for a clean producer exit it does not.

Which of these is correct depends on the intended lifetime contract for the extension seam, so it needs a maintainer's call before implementation. Happy to implement whichever direction is chosen. Scope is bounded: tensor-pool is opt-in (--features tensor-pool), off by default, and outside the 1.0 compatibility guarantees.

---

Generated by Claude Code