Skip to content

Commit 0a8d086

Browse files
apapirovskiBethGriggs
authored andcommitted
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: #22850 PR-URL: #21194 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]>
1 parent 3c8c53f commit 0a8d086

File tree

1 file changed

+9
-8
lines changed

1 file changed

+9
-8
lines changed

src/node_http2.cc

+9-8
Original file line numberDiff line numberDiff line change
@@ -548,8 +548,8 @@ Http2Session::~Http2Session() {
548548
ClearWrap(object());
549549
persistent().Reset();
550550
CHECK(persistent().IsEmpty());
551-
for (const auto& iter : streams_)
552-
iter.second->session_ = nullptr;
551+
for (const auto& stream : streams_)
552+
stream.second->session_ = nullptr;
553553
Unconsume();
554554
DEBUG_HTTP2SESSION(this, "freeing nghttp2 session");
555555
nghttp2_session_del(session_);
@@ -1782,11 +1782,11 @@ Http2Stream::Http2Stream(
17821782

17831783

17841784
Http2Stream::~Http2Stream() {
1785-
if (session_ != nullptr) {
1786-
DEBUG_HTTP2STREAM(this, "tearing down stream");
1787-
session_->RemoveStream(this);
1788-
session_ = nullptr;
1789-
}
1785+
if (session_ == nullptr)
1786+
return;
1787+
DEBUG_HTTP2STREAM(this, "tearing down stream");
1788+
session_->RemoveStream(this);
1789+
session_ = nullptr;
17901790

17911791
persistent().Reset();
17921792
CHECK(persistent().IsEmpty());
@@ -1862,7 +1862,8 @@ inline void Http2Stream::Destroy() {
18621862
// We can destroy the stream now if there are no writes for it
18631863
// already on the socket. Otherwise, we'll wait for the garbage collector
18641864
// to take care of cleaning up.
1865-
if (!stream->session()->HasWritesOnSocketForStream(stream))
1865+
if (stream->session() == nullptr ||
1866+
!stream->session()->HasWritesOnSocketForStream(stream))
18661867
delete stream;
18671868
}, this, this->object());
18681869

0 commit comments

Comments
 (0)