Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

5049 fix pipe race condition #5050

Open
wants to merge 2 commits into
base: 4.x
Choose a base branch
from

Conversation

NilsRenaud
Copy link
Contributor

Fixes the issue #5049

PipeTest#testFailStreamPrematurely was failing because the error handler is now activated before receiving the first element of the read stream.

But, I think that's an error in the FakeStream implementation which calls the failure handler immediately, bypassing the content delivery.
This means that the previous FakeThread implementation permitted:
- multiple failure handler calls on the same Stream
- A failure event THEN a new element read
This, IMO does not respect the ReadStream behaviour.

This test was only OK because the failure handler was set after the call to resume in the Pipe implementation. But it looks like a tweak to accommodate with the buggy FakeStream behavior.

Signed-off-by: Nils Renaud <[email protected]>
@NilsRenaud
Copy link
Contributor Author

NilsRenaud commented Jan 3, 2024

There is still a broken test RecordParserTest.testWrapReadStream, because a test explicitly calls FakeStream#write() after FakeStream#fail() I thought it wasn't possible...
Is this an error or are stream failures expected to happen multiple times on the same streams, even between 2 elements ?

@vietj vietj added this to the 4.5.2 milestone Jan 8, 2024
@NilsRenaud
Copy link
Contributor Author

@vietj what do you think of my previous comment, is RecordParserTest.testWrapReadStreambuggy ?

@vietj vietj modified the milestones: 4.5.2, 4.5.3 Jan 30, 2024
@vietj vietj modified the milestones: 4.5.3, 4.5.4 Feb 6, 2024
@vietj vietj modified the milestones: 4.5.4, 4.5.5 Feb 22, 2024
@vietj vietj modified the milestones: 4.5.5, 4.5.6 Mar 14, 2024
@vietj vietj modified the milestones: 4.5.6, 4.5.7, 4.5.8 Mar 21, 2024
@vietj vietj modified the milestones: 4.5.8, 4.5.9 May 24, 2024
@vietj vietj modified the milestones: 4.5.9, 4.5.10 Jul 17, 2024
@vietj vietj modified the milestones: 4.5.10, 4.5.11 Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants