Skip to content

Commit

Permalink
http2: wait for session to finish writing before destroy
Browse files Browse the repository at this point in the history
Backport-PR-URL: #34845
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 authored and MylesBorins committed Nov 16, 2020
1 parent 95d403a commit 2471197
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 8 deletions.
14 changes: 6 additions & 8 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1022,6 +1022,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 @@ -1390,16 +1392,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
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

0 comments on commit 2471197

Please sign in to comment.