From c8a00fdf62ff2645fe4be515811429d08ba3d641 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 24 Oct 2017 23:41:48 +0200
Subject: [PATCH] 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>
---
 src/node_http2.cc         | 15 ++++++++++-----
 src/node_http2.h          |  2 ++
 src/node_http2_core-inl.h | 12 +++++++-----
 src/node_http2_core.h     |  6 +++++-
 4 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/node_http2.cc b/src/node_http2.cc
index 9f5273c550600a..744d55078501a8 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 65cb12578fe071..1006dff3ced70b 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 11f48381ef22ca..fa365fee059b4d 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 8724b03e76cba6..2e885b73dab8f5 100644
--- a/src/node_http2_core.h
+++ b/src/node_http2_core.h
@@ -97,7 +97,7 @@ class Nghttp2Session {
       nghttp2_mem* mem = nullptr);
 
   // Frees this session instance
-  inline int Free();
+  inline ~Nghttp2Session();
   inline void MarkDestroying();
   bool IsDestroying() {
     return destroying_;
@@ -140,6 +140,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_;
   }
@@ -201,6 +203,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);