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

Verify processing of stream control frames after reset #1598

Merged
merged 13 commits into from
Dec 18, 2023

Conversation

huitema
Copy link
Collaborator

@huitema huitema commented Dec 17, 2023

This PR tests three kinds of events that can involve "Reset Stream", "Max stream data" and "Stop Sending" frames processed after a stream has been reset or otherwise closed:

  • processing the ACK of a packet that contained the frame, because
    the ACK may be received after the reset after the stream context
    was deleted,
  • receiving extra copies of the frames after the stream is deleted.
    These extra copies shall be ignored with no side effect.
  • processing of frames in packets detected as lost after the
    stream was deleted. The stack queries whether the packets needs to
    be repeated.

The combination of "ack/extra/need" with three frame types produces
9 possible tests. However, there is no acking processing for
stop sending frames, so we do not implement a test for
"ack of a stop sending frame.

Issue #1597 describes one such event: the stack tries to process the acknowledgement of a "reset stream" frame after the stream has been deleted. This issue is fixed in this PR, along several similar issues detected by the new tests.

Close #1597

Copy link
Collaborator

@suhasHere suhasHere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks greT

@huitema huitema merged commit f885cbb into master Dec 18, 2023
11 checks passed
@huitema huitema deleted the reset-race-condition branch December 18, 2023 19:47
@TimEvens
Copy link

@huitema , thanks for getting this fix in so quickly

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.

Possible race condition with stream reset causing PICOQUIC_TRANSPORT_STREAM_STATE_ERROR
3 participants