From 247119709916970885d0f718feae2a1bcb763e0f Mon Sep 17 00:00:00 2001
From: Denys Otrishko <shishugi@gmail.com>
Date: Sat, 7 Dec 2019 19:04:12 +0200
Subject: [PATCH] http2: wait for session to finish writing before destroy

Backport-PR-URL: https://github.com/nodejs/node/pull/34845
PR-URL: https://github.com/nodejs/node/pull/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>
---
 lib/internal/http2/core.js | 14 ++++++--------
 src/node_http2.cc          | 13 +++++++++++++
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 7515227fa82be4..a7f963f221b326 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -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);
@@ -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:
diff --git a/src/node_http2.cc b/src/node_http2.cc
index f2b9f40a04c0b4..d317d1f32ccaba 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -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.
@@ -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();