-
Notifications
You must be signed in to change notification settings - Fork 301
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
Await connection and flush before closing socket. #1532
Conversation
634a33a
to
f19b959
Compare
@jasnell should be ready to go |
be83106
to
9e00f8b
Compare
src/workerd/api/streams/internal.h
Outdated
@@ -143,6 +147,10 @@ class ReadableStreamInternalController: public ReadableStreamController { | |||
bool disturbed = false; | |||
bool readPending = false; | |||
|
|||
// Used by Sockets code to signal to the ReadableStream that it should error when read from | |||
// because the socket is currently being closed. | |||
bool errorOnReadsDueToPendingClosure = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other idea is to stick it into the StreamState but there seems to be a bunch of code that checks for !StreamState.is<Unlocked>
and I didn't want to mess with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but with a couple comments I'd prefer to see addressed first.
9e00f8b
to
fd8d741
Compare
fd8d741
to
9b89c8a
Compare
Repeat of https://github.com/cloudflare/workerd/pull/1369/files with some changes.
Behind an autogate so we can roll this out gradually.
Tested upstream.