Fix fair_queue tests to match disconnect behavior#220
Merged
Conversation
PR #207 changed FairQueue to return None immediately when any stream ends (to properly handle disconnects). The tests from PR #213 were written with the old behavior in mind where the queue would continue until all streams finished. Updated tests to expect None when the first stream ends rather than waiting for all streams to complete or return Pending.
PR #207 changed fair_queue to return None immediately when any stream ends. This broke multi-client scenarios like REP sockets with many concurrent clients - when the first client disconnected, the server would stop receiving from all remaining clients. This commit reverts to the previous behavior: when a stream ends, continue polling other streams. Only return None when truly all streams are exhausted. Changes: - fair_queue: continue polling after stream disconnect - Updated tests to expect all streams to be consumed - REP socket: restored None case handling for when queue is empty
Member
Author
|
Ok so this surfaced an issue on a REP socket with 100 concurrent clients. The first client to finish would cause the server to stop receiving from the remaining 99 clients. |
The test was written correctly - it expected Pending when a stream ends but other streams remain. The implementation change in PR #207 broke this expectation. Our fix restores the behavior the test expected.
Member
Author
|
This basically reverts #207 in the end. Sadly multiclients were broken by the None propagation from the fair queue disconnects. |
poyea
approved these changes
Dec 18, 2025
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.
PR #207 changed FairQueue to return None immediately when any stream ends (to properly handle disconnects). The tests from PR #213 were written with the old behavior in mind where the queue would continue until all streams finished.
Updated tests to expect None when the first stream ends rather than waiting for all streams to complete or return Pending.
cc @jbbjarnason