http2: make sessions garbage-collectible

Use weak handles to track the lifetime of an `Http2Session` instance.

Refs: https://github.com/nodejs/http2/issues/140
PR-URL: https://github.com/nodejs/node/pull/16461
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>

4 files changed, 24 insertions, 11 deletions

diff --git a/src/node_http2.cc b/src/node_http2.cc
index 9f5273c550..744d550785 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -75,7 +75,7 @@ Http2Session::Http2Session(Environment* env,
                            nghttp2_session_type type)
     : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_HTTP2SESSION),
       StreamBase(env) {
-  Wrap(object(), this);
+  MakeWeak<Http2Session>(this);
 
   Http2Options opts(env);
 
@@ -102,11 +102,16 @@ Http2Session::Http2Session(Environment* env,
 }
 
 Http2Session::~Http2Session() {
-  CHECK_EQ(false, persistent().IsEmpty());
-  ClearWrap(object());
+  CHECK(persistent().IsEmpty());
+  Close();
+}
+
+void Http2Session::Close() {
+  if (!object().IsEmpty())
+    ClearWrap(object());
   persistent().Reset();
-  CHECK_EQ(true, persistent().IsEmpty());
 
+  this->Nghttp2Session::Close();
   // Stop the loop
   CHECK_EQ(uv_prepare_stop(prep_), 0);
   auto prep_close = [](uv_handle_t* handle) {
@@ -407,7 +412,7 @@ void Http2Session::Destroy(const FunctionCallbackInfo<Value>& args) {
 
   if (!skipUnconsume)
     session->Unconsume();
-  session->Free();
+  session->Close();
 }
 
 void Http2Session::Destroying(const FunctionCallbackInfo<Value>& args) {
diff --git a/src/node_http2.h b/src/node_http2.h
index 65cb12578f..1006dff3ce 100644
--- a/src/node_http2.h
+++ b/src/node_http2.h
@@ -474,6 +474,8 @@ class Http2Session : public AsyncWrap,
     return stream_buf_;
   }
 
+  void Close() override;
+
  private:
   StreamBase* stream_;
   StreamResource::Callback<StreamResource::AllocCb> prev_alloc_cb_;
diff --git a/src/node_http2_core-inl.h b/src/node_http2_core-inl.h
index 11f48381ef..fa365fee05 100644
--- a/src/node_http2_core-inl.h
+++ b/src/node_http2_core-inl.h
@@ -587,16 +587,18 @@ inline void Nghttp2Session::MarkDestroying() {
   destroying_ = true;
 }
 
-inline int Nghttp2Session::Free() {
-#if defined(DEBUG) && DEBUG
-  CHECK(session_ != nullptr);
-#endif
+inline Nghttp2Session::~Nghttp2Session() {
+  Close();
+}
+
+inline void Nghttp2Session::Close() {
+  if (IsClosed())
+    return;
   DEBUG_HTTP2("Nghttp2Session %s: freeing session\n", TypeName());
   nghttp2_session_terminate_session(session_, NGHTTP2_NO_ERROR);
   nghttp2_session_del(session_);
   session_ = nullptr;
   DEBUG_HTTP2("Nghttp2Session %s: session freed\n", TypeName());
-  return 1;
 }
 
 // Write data received from the socket to the underlying nghttp2_session.
diff --git a/src/node_http2_core.h b/src/node_http2_core.h
index 66fa91f628..21df45c42c 100644
--- a/src/node_http2_core.h
+++ b/src/node_http2_core.h
@@ -98,7 +98,7 @@ class Nghttp2Session {
       nghttp2_mem* mem = nullptr);
 
   // Frees this session instance
-  inline int Free();
+  inline ~Nghttp2Session();
   inline void MarkDestroying();
   bool IsDestroying() {
     return destroying_;
@@ -141,6 +141,8 @@ class Nghttp2Session {
   // Returns the nghttp2 library session
   inline nghttp2_session* session() const { return session_; }
 
+  inline bool IsClosed() const { return session_ == nullptr; }
+
   nghttp2_session_type type() const {
     return session_type_;
   }
@@ -202,6 +204,8 @@ class Nghttp2Session {
 
   virtual uv_loop_t* event_loop() const = 0;
 
+  virtual void Close();
+
  private:
   inline void HandleHeadersFrame(const nghttp2_frame* frame);
   inline void HandlePriorityFrame(const nghttp2_frame* frame);