yknoya commented on code in PR #12936:
URL: https://github.com/apache/trafficserver/pull/12936#discussion_r2928207890
##########
src/proxy/http2/Http2ConnectionState.cc:
##########
@@ -477,13 +498,19 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame
&frame)
Http2ErrorCode result =
stream->decode_header_blocks(*this->local_hpack_handle,
this->acknowledged_local_settings.get(HTTP2_SETTINGS_HEADER_TABLE_SIZE));
- // If this was an outbound connection and the state was already closed,
just clear the
- // headers after processing. We just processed the heaer blocks to keep
the dynamic table in
+ // We just processed the heaer blocks to keep the dynamic table in
// sync with peer to avoid future HPACK compression errors
if (reset_header_after_decoding) {
stream->reset_receive_headers();
+ SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread());
+ this->stream_list.remove(stream);
if (free_stream_after_decoding) {
- THREAD_FREE(stream, http2StreamAllocator, this_ethread());
+ stream->initiating_close();
+ }
Review Comment:
As a conclusion, I added a `suppress_rst` argument to the `initiating_close`
method so that sending an RST_STREAM frame can be suppressed:
https://github.com/apache/trafficserver/pull/12936/commits/d98f026c077981ffce20386f2098bf1970b239cd
Calling `THREAD_FREE` directly would be simpler. However, this is not
feasible because it triggers the assertion in the `Http2Stream` destructor
shown below, which results in a crash:
https://github.com/apache/trafficserver/blob/8d5a71f5e8a1686ff6edcf53935e10add5f70441/src/proxy/http2/Http2Stream.cc#L101
Regarding the alternative approach of modifying the stream state before
calling `initiating_close`, this relies on the assumption that an RST_STREAM
frame will be sent later in the subsequent processing flow. If that assumption
changes in the future, it could introduce another issue. Therefore, I decided
not to adopt that approach.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
