Fix publisher attach corrupting resized buffer metadata#103
Closed
cursor[bot] wants to merge 2 commits into
Closed
Fix publisher attach corrupting resized buffer metadata#103cursor[bot] wants to merge 2 commits into
cursor[bot] wants to merge 2 commits into
Conversation
Co-authored-by: dallison <dallison@users.noreply.github.com>
Co-authored-by: dallison <dallison@users.noreply.github.com>
This was referenced Jul 7, 2026
dallison
added a commit
that referenced
this pull request
Jul 16, 2026
When a channel was resized and a later publisher attached to the existing buffers, both the joining publisher and the server-reported channel info could observe the smaller joining slot size instead of the resized one. Two bugs combined: - CreateOrAttachBuffers recorded attached-buffer sizes at bcb_->sizes[buffers_.size()] *after* push_back (off-by-one, clobbering the next buffer's size slot) and stored the joining publisher's requested size rather than the buffer's real size. The MEMFD path also treated an opened, already-registered buffer like a freshly created one. Record the real buffer size under its own buffer_index and explicitly distinguish attaching from creating for all shm backends. - On POSIX, CreatePosixSharedMemoryFile unconditionally ftruncate'd the shadow file (whose size is the authoritative record of a buffer's size) to the caller's requested size. A publisher attaching with a smaller slot size therefore shrank a buffer an earlier publisher had created larger, so GetBufferSize later read back the shrunken size. Size the shadow file only when it has not been sized yet, so attaching publishers never resize an existing buffer. The server derives channel slot size from bcb_->sizes[num_buffers-1], so both bugs also skewed GetChannelInfo. Added a regression test that resizes a publisher, attaches a second publisher from another client, and checks both the second publisher and the server channel info keep the resized 512-byte slot size. Supersedes cursor bot PR #103, which fixed only the index/MEMFD half and did not pass on the POSIX shared-memory backend.
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
2 participants
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.
Bug and impact
When a publisher resized a channel and a later publisher attached to the existing buffers, the attach path updated the wrong BCB size slot. This could make the server report the resized channel's latest slot size as the smaller joining publisher size, breaking channel info, bridge negotiation, and subsequent clients.
Root cause
PublisherImpl::CreateOrAttachBufferspushed an attachedBufferSetbefore writingbcb_->sizes[buffers_.size()], which targeted the next buffer index. The memfd path also treated opened existing descriptors like newly-created buffers and used the joining publisher's requested size instead of the fd's real size.Fix
Record attached buffers under
buffer_indexusingGetBufferSize, and explicitly distinguish existing-buffer opens from new buffer creation for both named shm and memfd backends. Added a regression that resizes a publisher, attaches a second publisher, and verifies both the second publisher and server channel info preserve the resized 512-byte slot size.Validation
npx --yes @bazel/bazelisk test //client:client_test --test_filter=ClientTest.AttachingPublisherPreservesResizedSlotSizenpx --yes @bazel/bazelisk test //client:client_test --config=linux_memfd --test_filter=ClientTest.AttachingPublisherPreservesResizedSlotSize