Skip to content

[v12.x] http2: use and support non-empty DATA frame with END_STREAM flag #34845

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
clshortfuse wants to merge 5 commits into from
Closed

[v12.x] http2: use and support non-empty DATA frame with END_STREAM flag #34845

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4e4592b
http2: wait for session to finish writing before destroy
lundibundi Dec 7, 2019
0453ece
http2: wait for session socket writable end on close/destroy
lundibundi Dec 8, 2019
98eea4f
http2,doc: minor fixes
mildsunrise Apr 27, 2020
6802870
Revert "http2: streamline OnStreamRead streamline memory accounting"
Trott Jul 11, 2020
a9ea96c
http2: use and support non-empty DATA frame with END_STREAM flag
clshortfuse Jun 14, 2020
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
Next Next commit
http2: wait for session to finish writing before destroy 
PR-URL: #30854
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
@lundibundi @clshortfuse
lundibundi authored and clshortfuse committed Sep 22, 2020
commit 4e4592bf254d153b9fdf2235f8c6792f44de0e9c
14 changes: 6 additions & 8 deletions lib/internal/http2/core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1020,6 +1020,8 @@ function emitClose(self, error) {
}

function finishSessionDestroy(session, error) {
debugSessionObj(session, 'finishSessionDestroy');

const socket = session[kSocket];
if (!socket.destroyed)
socket.destroy(error);
Expand Down Expand Up @@ -1388,16 +1390,12 @@ class Http2Session extends EventEmitter {
const handle = this[kHandle];

// Destroy the handle if it exists at this point
if (handle !== undefined)
if (handle !== undefined) {
handle.ondone = finishSessionDestroy.bind(null, this, error);
handle.destroy(code, socket.destroyed);

// If the socket is alive, use setImmediate to destroy the session on the
// next iteration of the event loop in order to give data time to transmit.
// Otherwise, destroy immediately.
if (!socket.destroyed)
setImmediate(finishSessionDestroy, this, error);
else
} else {
finishSessionDestroy(this, error);
}
}

// Closing the session will:
Expand Down
13 changes: 13 additions & 0 deletions src/node_http2.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,13 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {

flags_ |= SESSION_STATE_CLOSED;

// If we are writing we will get to make the callback in OnStreamAfterWrite.
if ((flags_ & SESSION_STATE_WRITE_IN_PROGRESS) == 0) {
Debug(this, "make done session callback");
HandleScope scope(env()->isolate());
MakeCallback(env()->ondone_string(), 0, nullptr);
}

// If there are outstanding pings, those will need to be canceled, do
// so on the next iteration of the event loop to avoid calling out into
// javascript since this may be called during garbage collection.
Expand Down Expand Up @@ -1530,6 +1537,12 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
stream_->ReadStart();
}

if ((flags_ & SESSION_STATE_CLOSED) != 0) {
HandleScope scope(env()->isolate());
MakeCallback(env()->ondone_string(), 0, nullptr);
return;
}

// If there is more incoming data queued up, consume it.
if (stream_buf_offset_ > 0) {
ConsumeHTTP2Data();
Expand Down