http2: make sessions garbage-collectible

Use weak handles to track the lifetime of an `Http2Session` instance. Refs: nodejs/http2#140 PR-URL: #16461 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
nodejs · Nov 6, 2017 · bf7dc38 · bf7dc38
1 parent 3f52962
commit bf7dc38
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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);