diff options
author | Akshay K <iit.akshay@gmail.com> | 2021-07-31 01:46:45 +0300 |
---|---|---|
committer | Beth Griggs <bgriggs@redhat.com> | 2021-08-06 16:39:55 +0300 |
commit | 60fd5d4be14eb81799cca88a0b2d931dedeef1d6 (patch) | |
tree | 7fa06525abc2b1392a706ebfed015e7b785dff10 /src/node_http2.cc | |
parent | c7a0f1ddce2d6835b9f69c5d0da39c1ab39928b7 (diff) |
http2: update handling of rst_stream with error code NGHTTP2_CANCEL
The PR updates the handling of rst_stream frames and adds all streams
to the pending list on receiving rst frames with the error code
NGHTTP2_CANCEL.
The changes will remove dependency on the stream state that may allow
bypassing the checks in certain cases. I think a better solution is to
delay streams in all cases if rst_stream is received for the cancel
events.
The rst_stream frames can be received for protocol/connection error as
well it should be handled immediately. Adding streams to the pending
list in such cases may cause errors.
PR-URL: https://github.com/nodejs/node/pull/39622
Refs: https://github.com/nodejs/node/pull/39423
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Diffstat (limited to 'src/node_http2.cc')
-rw-r--r-- | src/node_http2.cc | 24 |
1 files changed, 12 insertions, 12 deletions
diff --git a/src/node_http2.cc b/src/node_http2.cc index 625f3adb3b9..c01fa4d5255 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -2196,21 +2196,21 @@ void Http2Stream::SubmitRstStream(const uint32_t code) { CHECK(!this->is_destroyed()); 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 + auto is_stream_cancel = [](const uint32_t code) { + return code == NGHTTP2_CANCEL; + }; + + // If RST_STREAM frame is received with error code NGHTTP2_CANCEL, + // add it to the pending list and don't force purge the data. It is + // to avoids the double free error due to unwanted behavior of nghttp2. + + // Add stream to the pending list only 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() && - !is_writable() && is_reading()) { - session_->AddPendingRstStream(id_); - return; + if (session_->is_in_scope() && is_stream_cancel(code)) { + session_->AddPendingRstStream(id_); + return; } |