From 0a8d0861f2165aa1a6139fb3a073a7b41312aa02 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Tue, 5 Jun 2018 20:34:47 -0400 Subject: [PATCH] http2: safer Http2Session destructor It's hypothetically (and with certain V8 flags) possible for the session to be garbage collected before all the streams are. In that case, trying to remove the stream from the session will lead to a segfault due to attempting to access no longer valid memory. Fix this by unsetting the session on any streams still around when destroying. Backport-PR-URL: https://github.com/nodejs/node/pull/22850 PR-URL: https://github.com/nodejs/node/pull/21194 Reviewed-By: Anna Henningsen Reviewed-By: Ujjwal Sharma --- src/node_http2.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index 4eb35330fd42e9..139d622e36781b 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -548,8 +548,8 @@ Http2Session::~Http2Session() { ClearWrap(object()); persistent().Reset(); CHECK(persistent().IsEmpty()); - for (const auto& iter : streams_) - iter.second->session_ = nullptr; + for (const auto& stream : streams_) + stream.second->session_ = nullptr; Unconsume(); DEBUG_HTTP2SESSION(this, "freeing nghttp2 session"); nghttp2_session_del(session_); @@ -1782,11 +1782,11 @@ Http2Stream::Http2Stream( Http2Stream::~Http2Stream() { - if (session_ != nullptr) { - DEBUG_HTTP2STREAM(this, "tearing down stream"); - session_->RemoveStream(this); - session_ = nullptr; - } + if (session_ == nullptr) + return; + DEBUG_HTTP2STREAM(this, "tearing down stream"); + session_->RemoveStream(this); + session_ = nullptr; persistent().Reset(); CHECK(persistent().IsEmpty()); @@ -1862,7 +1862,8 @@ inline void Http2Stream::Destroy() { // We can destroy the stream now if there are no writes for it // already on the socket. Otherwise, we'll wait for the garbage collector // to take care of cleaning up. - if (!stream->session()->HasWritesOnSocketForStream(stream)) + if (stream->session() == nullptr || + !stream->session()->HasWritesOnSocketForStream(stream)) delete stream; }, this, this->object());