Skip to content

Correctly reset our state after .sendEnd #597

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

Merged
merged 2 commits into from
Jun 17, 2022

Conversation

Lukasa
Copy link
Collaborator

@Lukasa Lukasa commented Jun 17, 2022

Motivation

In some cases, the last thing that happens in a request-response pair is
that we send our HTTP/1.1 .end. This can happen when the peer has sent
an early response, before we have finished uploading our body. When it
does, we need to be diligent about cleaning up our connection state.

Unfortunately, there was an edge in the HTTP1ConnectionStateMachine that
processed .succeedRequest but that did not transition the state into
either .idle or .closing. That was an error, and needed to be fixed.

Modifications

Transition to .idle when we're returning
.succeedRequest(.sendRequestEnd).

Result

Fewer crashes
Resolves #595

Motivation

In some cases, the last thing that happens in a request-response pair is
that we send our HTTP/1.1 .end. This can happen when the peer has sent
an early response, before we have finished uploading our body. When it
does, we need to be diligent about cleaning up our connection state.

Unfortunately, there was an edge in the HTTP1ConnectionStateMachine that
processed .succeedRequest but that did not transition the state into
either .idle or .closing. That was an error, and needed to be fixed.

Modifications

Transition to .idle when we're returning
.succeedRequest(.sendRequestEnd).

Result

Fewer crashes
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Jun 17, 2022
@Lukasa Lukasa requested review from glbrntt and dnadoba June 17, 2022 08:17
Copy link
Collaborator

@dnadoba dnadoba left a comment

Choose a reason for hiding this comment

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

Great fix, thanks!

@Lukasa Lukasa merged commit 062989e into swift-server:main Jun 17, 2022
@Lukasa Lukasa deleted the cb-reset-state-after-send-end branch June 17, 2022 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crashing on precondition in HTTP1ConnectionStateMachine
3 participants