Skip to content

Harden SHM against retained zero-copy views (#272) - #275

Draft
cboulay wants to merge 2 commits into
cboulay/fingerprint-on-picklefrom
cboulay/shm-hardening
Draft

cboulay wants to merge 2 commits into
cboulay/fingerprint-on-picklefrom
cboulay/shm-hardening

Conversation

@cboulay

@cboulay cboulay commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Restacked: onto #274 / #268. The read-only SHM view now lives in #268's Channel._deliver_from_shm; the deferred-close commit applied unchanged.

Stacked on #274. Split from #273: the parts of the #272 fix that stand on their own. The ChannelFailed policy and the dead-segment race stay in draft #273.

Changes

  • Tolerant SHM close (shm.py): if SharedMemory.close() raises BufferError because views into the segment are still alive (a unit kept a zero-copy array), SHMContext drops its handle to the mmap and closes the fd. The mapping is unmapped when the last view is collected, and the GraphServer still unlinks the segment when its leases end. Before this, the channel died for good when the publisher grew its SHM. Relies on SharedMemory's private _mmap/_fd; close() is unchanged from Python 3.8 through 3.14.
  • Leak fix: SHMContext.wait_closed() also closes the segment. A monitor task cancelled before it first ran never executed its finally (the "Exception ignored in SharedMemory.__del__" noise at shutdown).
  • Grow race fixed: on a grow the publisher closed the old segment at once. A message already sent names it, and a channel that hadn't processed that message still had to attach it; if the publisher held the last lease, the GraphServer unlinked it first and the channel died with FileNotFoundError. Two quick grows lost this race whenever timing shifted (it broke Surface a dead channel to subscribers; refuse attaching unlinked SHM (#272) #273's race test, and later the existing grow test under Elide coordinate axes a receiving channel already holds #276). The publisher now retires the old segment with the last msg_id sent under its name and closes it once the backpressure wait for msg_id L + num_buffers is over. By then every channel has released message L, and a channel attaches the segment a message names before it can release it.
  • Read-only SHM messages: the channel caches shm[buf_idx].toreadonly(), matching the TCP path. Subscribers could previously write into memory shared with the publisher and every other subscriber.

Behaviour change

Editing a received array in place (msg.data *= 2) now fails with "assignment destination is read-only" on SHM deliveries, as it already did over TCP. Worth a release note for the beta.

Not fixed here

A message kept past its callback is still overwritten when the publisher reuses its slot. This PR only stops the crash; units that keep received data must copy it. The baseproc hash witness, which did this implicitly, is fixed in ezmsg-org/ezmsg-baseproc#18.

Tests

  • test_shm.py::test_close_with_live_view
  • test_shm_grow.py::test_cross_process_grow_with_retained_views: a receiver keeps every message across two grows. Without the fix it hangs.

Full suite against a private GraphServer: only the 10 test_settings_json_schema / test_inspect_command failures that also fail on the base branch. test_shm_resize_race_repro_completes passes, and the grow tests passed 8/8 under the timing that used to lose the race.

Cost: the read-only view is ~20 ns/msg; the tolerant close runs only when a segment is closed.

cboulay added a commit that referenced this pull request Oct 1, 2026
…272)

What remains of the #272 work after the uncontroversial parts moved to
#275, kept here for review of the policy it implies.

- Channel: a crashed publisher connection records its exception and
  notifies its clients; Subscriber.recv_zero_copy raises ChannelFailed
  (chained from the cause) instead of waiting forever. handle_subscriber
  logs it at ERROR and keeps serving the stream's other publishers.
- GraphServer: a segment whose last lease has ended is marked unlinked and
  forgotten, so a late SHM_ATTACH is refused (ValueError, which channels
  already treat as a stale generation) instead of handing back a name the
  client cannot open. Unlinked entries are also pruned on SHM_CREATE, and a
  segment is never unlinked twice.
- Channel: FileNotFoundError on attach is treated as stale too, for older
  GraphServers.
cboulay added a commit that referenced this pull request Oct 1, 2026
…272)

What remains of the #272 work after the uncontroversial parts moved to
#275, kept here for review of the policy it implies.

- Channel: a crashed publisher connection records its exception and
  notifies its clients; Subscriber.recv_zero_copy raises ChannelFailed
  (chained from the cause) instead of waiting forever. handle_subscriber
  logs it at ERROR and keeps serving the stream's other publishers.
- GraphServer: a segment whose last lease has ended is marked unlinked and
  forgotten, so a late SHM_ATTACH is refused (ValueError, which channels
  already treat as a stale generation) instead of handing back a name the
  client cannot open. Unlinked entries are also pruned on SHM_CREATE, and a
  segment is never unlinked twice.
- Channel: FileNotFoundError on attach is treated as stale too, for older
  GraphServers.
@cboulay
cboulay marked this pull request as draft October 1, 2026 15:58
Split from #273: the parts that stand on their own, without the
ChannelFailed policy that is still under discussion.

- SHMContext: if closing the mapping raises BufferError because views into
  it are still alive (a unit kept a zero-copy array), drop the handle and
  let the mmap be unmapped when the last view is collected, instead of
  killing the channel when the publisher grows its segment.
- SHMContext.wait_closed() also closes the segment: a monitor task
  cancelled before it first ran never executed its finally, leaking the
  mapping (the "Exception ignored in SharedMemory.__del__" noise).
- Channel: SHM-delivered messages are read-only, matching the TCP path, so
  a subscriber cannot write into memory shared with the publisher and
  every other subscriber.
- Tests: closing with a live view, and a cross-process grow with a
  receiver that retains every message.
On a grow the publisher closed the old segment at once. A message already
sent names that segment, and a channel that has not processed it yet still
has to attach it; if the publisher's lease was the last one, the GraphServer
unlinked it first and the channel's attach failed with FileNotFoundError,
killing the channel. Two quick grows (num_buffers > 1, no wait for acks
between them) lost that race whenever timing shifted.

The publisher now retires the old segment with the last msg_id sent under
its name, and closes it once the backpressure wait for msg_id L +
num_buffers is over: by then every channel has released message L, and a
channel attaches the segment a message names before it can release it.

This branch has not been deployed

No deployments
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.

1 participant