Skip to content

Commit

Permalink
http2: make sessions garbage-collectible
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
addaleax authored and MylesBorins committed Oct 31, 2017
1 parent bfc1ad0 commit 54ebf91
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 11 deletions.
15 changes: 10 additions & 5 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions src/node_http2.h
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,8 @@ class Http2Session : public AsyncWrap,
return stream_buf_;
}

void Close() override;

private:
StreamBase* stream_;
StreamResource::Callback<StreamResource::AllocCb> prev_alloc_cb_;
Expand Down
12 changes: 7 additions & 5 deletions src/node_http2_core-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion src/node_http2_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down Expand Up @@ -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_;
}
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 54ebf91

Please sign in to comment.