Repository navigation
Fix waiting for the data channel send buffer to drain - #1267
Draft
hiroshihorie wants to merge 3 commits into
Draft
hiroshihorie wants to merge 3 commits into
hiroshihorie wants to merge 3 commits into
Conversation
waitForBufferStatusLow had three problems: - It polled only the cached buffer status, which changes on a send or on the channel's low-buffer event. When a full reconnect replaced a backed up channel, the new one fired neither, so a waiter could hang until disconnect. The cache is now reset when the channels are created, and the wait also checks the live channel. - When the engine closed during a wait, the wait resolved normally and the caller went on to send. It now fails. - Every wait that blocked left an EngineClosingEvent listener behind until the engine closed. The listener is now cancelled. MockDataChannel gains fakeBufferedAmount to simulate a channel that has not drained.
…-paths' into hiroshi/fix-buffer-status-wait # Conflicts: # lib/src/core/engine.dart
Every terminal path now marks the engine closed synchronously, so the wait can check that state directly. This drops the listener the wait had to remove again, and a wait started after closure fails the same way.
hiroshihorie
changed the base branch from
main
to
hiroshi/close-engine-on-terminal-paths
October 29, 2026 20:11
This branch has not been deployed
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.
Stacked on #1268. Review only the last commit, bb1c5b8. The merge commit before it brings in #1268.
Engine.waitForBufferStatusLowis the backpressure wait for data stream writes. It had three bugs:'Engine disconnected'.EngineClosingEventlistener was never cancelled. The wait no longer uses a listener at all.Bugs 2 and 3 are fixed by checking the engine's closed state in the wait loop instead of listening for
EngineClosingEvent. That only works because #1268 marks the engine closed synchronously on every terminal path, including room disposal and a reconnect that gives up. Hence the stack.This matters more with #1166, where every native data stream packet goes through this wait.
Tests
test/core/buffer_status_wait_test.darthas one test per bug. All three fail on #1268 and onmain, and pass here.MockDataChannelgainsfakeBufferedAmount, identical to #1266's, so the two merge cleanly. analyze, format, import_sorter, version check andflutter testall pass.E2E (macOS, local
livekit-server --dev)A local server drains the channel faster than the 2 MB low-water mark, so these runs used test-only builds with the mark lowered to 32 KB (not in this PR):
mainOn macOS the old channel's low-buffer event during teardown releases the waiter, so bug 1 doesn't show there. Other platforms are unchecked.
Merge order
#1264, then #1268, then this PR. Once #1268 merges, this PR retargets to
mainand I'll mergemainback in.