Repository navigation
Store POSIX buffer size in shadow file contents, not its length - #138
Merged
Merged
Conversation
The POSIX shared-memory backend (macOS, QNX) records each buffer's true size in a /tmp shadow file because fstat on the shm object is page-aligned. The size was recorded by ftruncating the shadow file to the full buffer size, on the assumption that it would be sparse. On QNX /tmp is /dev/shmem, so the shadow committed as much RAM as the buffer itself and every copy buffer cost 2x its size. Write a fixed 16-byte record (magic + uint64 size) into the shadow file instead, with a single write() so readers see either nothing or the whole record. The magic starts with NUL so server cleanup never parses a shadow file as split-buffer metadata. Readers fall back to the file length when there is no record, which covers a not-yet-written file and shadows sized by older clients. The Rust client had its own copy of this logic and is updated to the same format; it now also only writes into an empty shadow file, matching the C++ client. Also unlink the shm object (not the shadow path) when ftruncate of the shm object fails.
Previously every publisher creating a buffer wrote its requested size to the shadow file if the file was empty, and then raced on shm_open(O_CREAT | O_EXCL). Two publishers could both see an empty shadow; the last writer's size stayed in the shadow while the O_EXCL winner sized the object, so the loser could map more than the object holds. Now only the publisher that wins shm_open(O_CREAT | O_EXCL) writes the record, overwriting any stale record, and it does so before sizing the shm object. Publishers that lose wait (bounded, ~1s) until the object has a nonzero size and then read the record, which is then guaranteed to be the creator's. Readers no longer fall back to a shadow length of 16 bytes or less: a missing record in a file that small means it is not written yet, while shadows sized by older clients are always larger. Rust client: same ownership rule, create_shm distinguishes "already exists" from real errors, and an attaching publisher now records the buffer's real size in the BCB rather than its requested size (the C++ client already did this).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On the POSIX shared-memory backend (macOS, QNX),
CreatePosixBufferfirst creates/tmp/subspace_<session>_<channel>_<n>and ftruncates it to the full buffer size, then creates the real shm object at the same size. The comment assumed the shadow "will not actually allocate any disk space", but on QNX/tmpis/dev/shmem, so the shadow commits RAM.Measured on QNX (c4_1m): model data buffers 32.01 MiB, measured shm 64.01 MiB (exactly 2.000×). All 8 copy scenarios matched
2 × N × (size + 192)within 0.001 MiB, and a 64 MiB sparse file in/tmpraised system RAM by 64.7 MiB.Fix
Size stored as data, not file length. The shadow file holds a fixed 16-byte record (8-byte magic + native-endian
uint64size), written with a singlewrite(). Only the real shm object is sized to the buffer. The magic starts with a NUL byte: the server's/tmpcleanup tries to parsesubspace_*files as split-buffer metadata protos first, and a tag byte of 0 never parses.The shm creator owns the size. Previously every creating publisher wrote its requested size into an empty shadow and then raced on
shm_open(O_CREAT | O_EXCL), so two publishers could leave the shadow holding the loser's size while the winner sized the object (this race also existed with the old ftruncate code). Now:O_EXCLwinner writes the record, overwriting any stale record, and it does so before sizing the shm object.Rust client. It had its own copy of this logic (same doubling bug) and now uses the same format and ownership rule.
create_shmnow distinguishes "already exists" from real errors, and an attaching Rust publisher records the buffer's real size in the BCB instead of its requested size (the C++ client already did this).Small adjacent fix. When ftruncate on the shm object fails, unlink the shm object name instead of the shadow file path, which was leaking the object.
Compatibility
New clients read old shadow files correctly. An old client reading a new shadow file would see a 16-byte buffer, so all C++ and Rust clients on a POSIX (QNX/macOS) host should be upgraded together.
Testing
ClientTest.PosixShadowFileDoesNotReserveBufferSize: 8 × 1 MiB slots; the shadow file stays under 4 KiB and a subscriber attaches with the full slot size.ClientTest.PosixShadowFileStaleSizeIsReplacedByCreator: plants a stale record before the publisher exists; the creator must replace it. Verified this test fails against the previous write-only-if-empty logic.bazelisk test //... --config=macos_arm64: all 27 test targets pass, including the C++/Rust cross-language tests.