fix: base LatestFrameReader liveness on segment magic (namespace-safe)#3
Merged
Merged
Conversation
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.
What
Fixes a cross-PID-namespace bug in
LatestFrameReader::is_writer_alive()— the same container/host class as the read-only-open fix (#2).The reader validated the producer with
platform::process_exists(writer_pid). Butwriter_pidis the producer's pid in its own PID namespace — adisplaysinkproducer runs as root inside a container, while the native-player consumer runs on the host. On the host, that container pid is a different process (or none), soprocess_exists()always returns false →is_writer_alive()reports the producer dead on every poll → the consumer's run loop churns into RECONNECTING between every frame (picture holds, but the status flickers).Fix
Liveness is now the segment magic alone — the writer publishes
LATEST_FRAME_MAGICon create and invalidates it on clean close, both directly in the shared segment, so it's namespace-independent:The
process_exists/get_process_start_timepid checks are removed from the reader. The writer-side stale-segment reclaim (reclaim_stale_or_throw) still usesprocess_exists— that runs in the writer's own namespace (a new producer reclaiming a dead producer's segment), where pids are comparable, so it's unchanged.writer_pid/writer_start_timestay in the header for that path.Trade-off (conscious)
Magic detects a clean close (writer invalidates magic) but not a producer that crashes without a clean close (magic stays valid). The old pid check nominally caught that — but it was already broken across the namespace boundary, so nothing is lost in the deployment that matters. Crash/hang detection for a non-cleanly-closed producer belongs in the consumer via frame staleness (frames stop advancing), which is likewise namespace-independent — a separate follow-up if needed.
Verification
cpp/build.shgreen;test_latest_frame18/18 pass — includingWriterGoneReportedNoCrash(clean writer destruction → magic invalidated → correctly reports gone) andWriterRestartReaderRecovers.latest_frame.huntouched → wire-compatible; streamer's pinned zerobuffer needs no rebuild, only native-player.🤖 Generated with Claude Code