[go: nahoru, domu]

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

Prevent queuing writes while internal writable is being piped #1861

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

jasnell
Copy link
Member
@jasnell jasnell commented Mar 19, 2024

In some uses we end up enqueuing a write event to a WritableStreamInternalController while it is being piped to, leading to an assertion given that we expect the Pipe event to be the last event in the queue while it is being piped. To match the behavior seen when using the WritableStreamDefaultWriter, we should reject attempts to write, close, or flush while the pipe is still going.

Also improves the logging around the assertion that was being hit and fixes another apparent latent bug in the pipe source close check.

@jasnell jasnell requested review from a team as code owners March 19, 2024 01:54
@jasnell

This comment was marked as outdated.

@jasnell jasnell force-pushed the jsnell/streams-internal-queue-assert-investigation branch from 135f304 to cd077e0 Compare March 19, 2024 02:10
src/workerd/api/streams/internal-test.c++ Outdated Show resolved Hide resolved
src/workerd/api/streams/internal.c++ Show resolved Hide resolved
src/workerd/api/streams/internal-test.c++ Show resolved Hide resolved
Previously, the internal writable would allow queuing additional
write events while it was being piped. This would lead to an assert
when the pipe finished because of the expectation that the Pipe event
is the last item in the queue. We can continue to write to the stream
once the piping is done as long as preventClose is true.
@jasnell jasnell force-pushed the jsnell/streams-internal-queue-assert-investigation branch from cd077e0 to 4d0af28 Compare March 19, 2024 14:03
@jasnell jasnell merged commit b042f69 into main Mar 19, 2024
10 checks passed
@jasnell jasnell deleted the jsnell/streams-internal-queue-assert-investigation branch March 19, 2024 14:58
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.

None yet

2 participants