From b263f2585ab53f56e0e22b46cf1f8519a8af8a05 Mon Sep 17 00:00:00 2001 From: Akshay K Date: Mon, 26 Jul 2021 08:21:51 -0400 Subject: [PATCH] http2: on receiving rst_stream with cancel code add it to pending list PR-URL: https://github.com/nodejs/node/pull/39423 Backport-PR-URL: https://github.com/nodejs/node/pull/39527 Fixes: https://github.com/nodejs/node/issues/38964 Reviewed-By: James M Snell Reviewed-By: Matteo Collina --- src/node_http2.cc | 17 +++++++++++++++++ src/node_http2.h | 16 ++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/node_http2.cc b/src/node_http2.cc index dec6d7dab9eed7..cc21373521e4c7 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -2150,6 +2150,23 @@ int Http2Stream::SubmitPriority(nghttp2_priority_spec* prispec, void Http2Stream::SubmitRstStream(const uint32_t code) { CHECK(!this->IsDestroyed()); code_ = code; + + // If RST_STREAM frame is received and stream is not writable + // because it is busy reading data, don't try force purging it. + // Instead add the stream to pending stream list and process + // the pending data when it is safe to do so. This is to avoid + // double free error due to unwanted behavior of nghttp2. + // Ref:https://github.com/nodejs/node/issues/38964 + + // Add stream to the pending list if it is received with scope + // below in the stack. The pending list may not get processed + // if RST_STREAM received is not in scope and added to the list + // causing endpoint to hang. + if (session_->is_in_scope() && IsReading()) { + session_->AddPendingRstStream(id_); + return; + } + // If possible, force a purge of any currently pending data here to make sure // it is sent before closing the stream. If it returns non-zero then we need // to wait until the current write finishes and try again to avoid nghttp2 diff --git a/src/node_http2.h b/src/node_http2.h index 045bdfd716da03..a59de18f920aca 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -764,6 +764,22 @@ class Http2Session : public AsyncWrap, return (flags_ & SESSION_STATE_CLOSED) || session_ == nullptr; } + + // The changes are backported and exposes APIs to check the + // status flag of `Http2Session` +#define IS_FLAG(name, flag) \ + bool is_##name() const { return flags_ & flag; } + + IS_FLAG(in_scope, SESSION_STATE_HAS_SCOPE) + IS_FLAG(write_scheduled, SESSION_STATE_WRITE_SCHEDULED) + IS_FLAG(closing, SESSION_STATE_CLOSING) + IS_FLAG(sending, SESSION_STATE_SENDING) + IS_FLAG(write_in_progress, SESSION_STATE_WRITE_IN_PROGRESS) + IS_FLAG(reading_stopped, SESSION_STATE_READING_STOPPED) + IS_FLAG(receive_paused, SESSION_STATE_NGHTTP2_RECV_PAUSED) + +#undef IS_FLAG + // Schedule a write if nghttp2 indicates it wants to write to the socket. void MaybeScheduleWrite();