maskit commented on code in PR #8963:
URL: https://github.com/apache/trafficserver/pull/8963#discussion_r941034088
##########
proxy/http2/Http2Stream.cc:
##########
@@ -435,23 +477,22 @@ Http2Stream::do_io_close(int /* flags */)
REMEMBER(NO_EVENT, this->reentrancy_count);
Http2StreamDebug("do_io_close");
+ // if (this->is_state_writeable()) { // Let the other end know we are
going away
+ // this->get_connection_state().send_rst_stream_frame(_id,
Http2ErrorCode::HTTP2_ERROR_NO_ERROR);
+ //}
+
// When we get here, the SM has initiated the shutdown. Either it
received a WRITE_COMPLETE, or it is shutting down. Any
// remaining IO operations back to client should be abandoned. The
SM-side buffers backing these operations will be deleted
// by the time this is called from transaction_done.
closed = true;
- if (_proxy_ssn && this->is_client_state_writeable()) {
- // Make sure any trailing end of stream frames are sent
- // We will be removed at send_data_frames or closing connection phase
- Http2ClientSession *h2_proxy_ssn = static_cast<Http2ClientSession
*>(this->_proxy_ssn);
- SCOPED_MUTEX_LOCK(lock, h2_proxy_ssn->mutex, this_ethread());
- h2_proxy_ssn->connection_state.send_data_frames(this);
- }
+ // Adjust state, so we don't process any more data
+ _state = Http2StreamState::HTTP2_STREAM_STATE_CLOSED;
Review Comment:
Not a blocker since there is already one place that does similar thing, but
I think we should not change `_state` like this without actual activity on H2
protocol, because we won't be able to know the last state of a stream, which
should be CLOSED state on a clean close even without this line.
--
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]
