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.

Sign up for GitHub

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

Jump to bottom

Bug: ReadableStream can be written to after close #27540

Closed
gnoff opened this issue Oct 18, 2023 · 0 comments · Fixed by #27541
Closed

Bug: ReadableStream can be written to after close #27540

gnoff opened this issue Oct 18, 2023 · 0 comments · Fixed by #27541
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@gnoff
Copy link
Collaborator

gnoff commented Oct 18, 2023

As reported in vercel/next.js#56919 it is possible for the ReadableStream controller to be written to after it is already closed. This is because inside an async context a Float method such as ReactDOM.preload() can attempt to flush newly created Resources even after the stream has completed.

React version:

The underlying bug has been in React since Float landed. The expression of the Bug as reported in the Next.js repo was landed in: 701ac2e
The first Canary release to have this new expression was: 18.3.0-canary-f81c0f1ed-20230927

Steps To Reproduce

  1. use react-dom/server.edge
  2. Ensure the environment supports AsyncLocalStorage
  3. Call Float method like ReactDOM.preload(...) after an await after the stream has closed

The current behavior

If the conditions are right the Float call will initiate a write after the stream has closed and there will be a thrown exception

The expected behavior

The Float call is ignored after the stream has closed. There are no errors
@gnoff gnoff added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Oct 18, 2023
@gnoff gnoff mentioned this issue Oct 18, 2023
Merged
gnoff added a commit that referenced this issue Oct 18, 2023
@gnoff
[Fizz][Float] Do not write after closing the stream (#27541)
601e5c3 
Float methods can hang on to a reference to a Request after the request
is closed due to AsyncLocalStorage. If a Float method is called at this
point we do not want to attempt to flush anything. This change updates
the closing logic to also call `stopFlowing` which will ensure that any
checks against the destination properly reflect that we cannot do any
writes. In addition it updates the enqueueFlush logic to existence check
the destination inside the work function since it can change across the
work scheduling gap if it is async.

fixes: #27540
github-actions bot pushed a commit that referenced this issue Oct 18, 2023
@gnoff
[Fizz][Float] Do not write after closing the stream (#27541)
24721ff 
Float methods can hang on to a reference to a Request after the request
is closed due to AsyncLocalStorage. If a Float method is called at this
point we do not want to attempt to flush anything. This change updates
the closing logic to also call `stopFlowing` which will ensure that any
checks against the destination properly reflect that we cannot do any
writes. In addition it updates the enqueueFlush logic to existence check
the destination inside the work function since it can change across the
work scheduling gap if it is async.

fixes: #27540

DiffTrain build for [601e5c3](601e5c3)
jerrydev0927 added a commit to jerrydev0927/react that referenced this issue Jan 5, 2024
@jerrydev0927
[Fizz][Float] Do not write after closing the stream (#27541)
49d17a1 
Float methods can hang on to a reference to a Request after the request
is closed due to AsyncLocalStorage. If a Float method is called at this
point we do not want to attempt to flush anything. This change updates
the closing logic to also call `stopFlowing` which will ensure that any
checks against the destination properly reflect that we cannot do any
writes. In addition it updates the enqueueFlush logic to existence check
the destination inside the work function since it can change across the
work scheduling gap if it is async.

fixes: facebook/react#27540

DiffTrain build for [601e5c38505ebc0ee099d8666b2f7a8b03159ac4](facebook/react@601e5c3)
EdisonVan pushed a commit to EdisonVan/react that referenced this issue Apr 15, 2024
@gnoff @AndyPengc12
[Fizz][Float] Do not write after closing the stream (facebook#27541)
13ac1d6 
Float methods can hang on to a reference to a Request after the request
is closed due to AsyncLocalStorage. If a Float method is called at this
point we do not want to attempt to flush anything. This change updates
the closing logic to also call `stopFlowing` which will ensure that any
checks against the destination properly reflect that we cannot do any
writes. In addition it updates the enqueueFlush logic to existence check
the destination inside the work function since it can change across the
work scheduling gap if it is async.

fixes: facebook#27540
gnoff added a commit to gnoff/react that referenced this issue Dec 20, 2024
@gnoff
[Fizz][Float] Do not write after closing the stream (facebook#27541)
5bbb04e 
Float methods can hang on to a reference to a Request after the request
is closed due to AsyncLocalStorage. If a Float method is called at this
point we do not want to attempt to flush anything. This change updates
the closing logic to also call `stopFlowing` which will ensure that any
checks against the destination properly reflect that we cannot do any
writes. In addition it updates the enqueueFlush logic to existence check
the destination inside the work function since it can change across the
work scheduling gap if it is async.

fixes: facebook#27540
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Fizz][Float] Do not write after closing the stream gnoff/react
1 participant
@gnoff