Disconnect evicted capture streams before dropping them - #69
Open
HoneyHazard wants to merge 1 commit into
Open
Conversation
StreamRegistry::add_stream() evicted an existing stream from its map without calling disconnect() on it first, unlike remove(), which already did. Dropping the evicted StreamRc alone destroys the client-side stream object (pw_stream_destroy) without ever calling pw_stream_disconnect, so the corresponding PipeWire node can be left registered in the graph until something else tears it down. This path is hit every time a capture is renewed after CaptureEligibility::NeedsRestart, since that calls start_capture() again for an object_id that already has an active stream.
HoneyHazard
marked this pull request as ready for review
August 6, 2026 23:15
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.
That being said, I hope these can be helpful and useful additions that users could appreciate.
Found this while working on a separate feature that counts
"wiremix-capture"nodes in the shared PipeWire graph - it kept seeing more capture nodes than wiremix's own bookkeeping (capturing_objects) accounted for, and it turned out to be a real leak, not a counting bug.StreamRegistry::add_stream()evicts any existing stream already registered under the samestream_idbefore inserting the new one:StreamRegistry::remove()does the same eviction, but calls.disconnect()on the outgoing stream first:add_stream()was missing that.disconnect()call. Dropping the evictedStreamRcalone only destroys the client-side stream object -StreamBox'sDropimpl callspw_stream_destroydirectly, neverpw_stream_disconnect. Without an explicit disconnect first, the corresponding node can be left registered in the graph, invisible to wiremix's own state but still visible to every other PipeWire client, until something else eventually tears it down.This path is hit every time a capture is renewed after
CaptureEligibility::NeedsRestart-start_capture()gets called again for anobject_idthat already has an active stream, soadd_stream()'s eviction branch runs on every restart, not just on genuinely new captures.Verified live (this module has no existing test coverage -
StreamRcrequires a real, connected PipeWire core to construct, nothing here is mocked):NeedsRestart), and watchedpw-dumpfor"wiremix-capture"-named nodes climb and stay elevated well past what the instance's owncapturing_objectssaid it was capturing.pw-dump's count tracked the instance's actual active capture count exactly, and clean shutdown (Ctrl+C) always returned the graph to baseline with nothing orphaned.cargo test --release: 144/144 passing (no regressions; this fix doesn't touch anything under existing test coverage)cargo fmt --check/cargo clippy -- -D warnings/cargo doc(matching this repo's CI): all clean