Skip to content

http2: fix end without read #20621

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

Closed
apapirovski wants to merge 2 commits into from
Closed

http2: fix end without read #20621

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c76cf39
http2: fix end without read
apapirovski May 9, 2018
504055f
fixup: add a comment
apapirovski May 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fixup: add a comment
  • Loading branch information
@apapirovski
apapirovski committed May 17, 2018
commit 504055fe7c49ee506d0be8afd1835a8ac4880c34
4 changes: 3 additions & 1 deletion lib/internal/http2/core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,9 @@ function onStreamClose(code) {
// Push a null so the stream can end whenever the client consumes
// it completely.
stream.push(null);

// If the client hasn't tried to consume the stream and there is no
// resume scheduled (which would indicate they would consume in the future),
// then just dump the incoming data so that the stream can be destroyed.
if (!stream[kState].didRead && !stream._readableState.resumeScheduled)
stream.resume();
Copy link
Copy Markdown
Member

@mcollina mcollina May 16, 2018

Choose a reason for hiding this comment

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

this is slightly different than read(0). Can you please explain why this is needed?
I would prefer if we didn't use _readableState here.

Copy link
Copy Markdown
Contributor Author

@apapirovski apapirovski May 16, 2018

Choose a reason for hiding this comment

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

This is equivalent to what we do in http and is needed because in cases where the user doesn't intend to consume the data (indicated by never having read and not having a pending resume call), we still want to be able to properly destroy the stream.

Copy link
Copy Markdown
Member

@mcollina mcollina May 16, 2018

Choose a reason for hiding this comment

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

Can you please add a comment about this?

Copy link
Copy Markdown
Contributor Author

@apapirovski apapirovski May 16, 2018

Choose a reason for hiding this comment

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

Will do later today.

}
Expand Down