Skip to content

Commit 2471197

Browse files
lundibundiMylesBorins
authored andcommittedNov 16, 2020
http2: wait for session to finish writing before destroy
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>
1 parent 95d403a commit 2471197

File tree

2 files changed

+19
-8
lines changed

2 files changed

+19
-8
lines changed
 

‎lib/internal/http2/core.js

+6-8
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,8 @@ function emitClose(self, error) {
10221022
}
10231023

10241024
function finishSessionDestroy(session, error) {
1025+
debugSessionObj(session, 'finishSessionDestroy');
1026+
10251027
const socket = session[kSocket];
10261028
if (!socket.destroyed)
10271029
socket.destroy(error);
@@ -1390,16 +1392,12 @@ class Http2Session extends EventEmitter {
13901392
const handle = this[kHandle];
13911393

13921394
// Destroy the handle if it exists at this point
1393-
if (handle !== undefined)
1395+
if (handle !== undefined) {
1396+
handle.ondone = finishSessionDestroy.bind(null, this, error);
13941397
handle.destroy(code, socket.destroyed);
1395-
1396-
// If the socket is alive, use setImmediate to destroy the session on the
1397-
// next iteration of the event loop in order to give data time to transmit.
1398-
// Otherwise, destroy immediately.
1399-
if (!socket.destroyed)
1400-
setImmediate(finishSessionDestroy, this, error);
1401-
else
1398+
} else {
14021399
finishSessionDestroy(this, error);
1400+
}
14031401
}
14041402

14051403
// Closing the session will:

‎src/node_http2.cc

+13
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,13 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
693693

694694
flags_ |= SESSION_STATE_CLOSED;
695695

696+
// If we are writing we will get to make the callback in OnStreamAfterWrite.
697+
if ((flags_ & SESSION_STATE_WRITE_IN_PROGRESS) == 0) {
698+
Debug(this, "make done session callback");
699+
HandleScope scope(env()->isolate());
700+
MakeCallback(env()->ondone_string(), 0, nullptr);
701+
}
702+
696703
// If there are outstanding pings, those will need to be canceled, do
697704
// so on the next iteration of the event loop to avoid calling out into
698705
// javascript since this may be called during garbage collection.
@@ -1530,6 +1537,12 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
15301537
stream_->ReadStart();
15311538
}
15321539

1540+
if ((flags_ & SESSION_STATE_CLOSED) != 0) {
1541+
HandleScope scope(env()->isolate());
1542+
MakeCallback(env()->ondone_string(), 0, nullptr);
1543+
return;
1544+
}
1545+
15331546
// If there is more incoming data queued up, consume it.
15341547
if (stream_buf_offset_ > 0) {
15351548
ConsumeHTTP2Data();

0 commit comments

Comments
 (0)
Please sign in to comment.