shinrich commented on a change in pull request #7845:
URL: https://github.com/apache/trafficserver/pull/7845#discussion_r661847536
##########
File path: proxy/http2/Http2ClientSession.cc
##########
@@ -711,3 +848,266 @@ Http2ClientSession::add_url_to_pushed_table(const char
*url, int url_len)
_h2_pushed_urls->emplace(url);
}
}
+
+/**
+ Handling critical HTTP/2 error cases like PROTOCOL_ERROR
+
+ 1). Send GOAWAY frame
+ 2). move state_goaway state
+ 3). indicate TXN(s) VC_EVENT_ERROR
+ */
+void
+Http2ClientSession::critical_error(Http2ErrorCode err)
+{
+ ink_release_assert(err != Http2ErrorCode::HTTP2_ERROR_NO_ERROR);
+
+ if (session_state == &Http2ClientSession::state_goaway || session_state ==
&Http2ClientSession::state_closed) {
+ return;
+ }
+
+ REMEMBER(NO_EVENT, recursion);
+ Http2SsnDebug("state_goaway by HTTP/2 error code=%d", (int)err);
+ _set_state_goaway(err);
+
+ // signal VC_EVENT_ERROR to txns
+ connection_state.cleanup_streams(VC_EVENT_ERROR);
+}
+
+/**
+ If there are no stream, add vc in the keep-alive queue
+
+ This may signal VC_EVENT_INACTIVITY_TIMEOUT and start closing VC and SSN
+ */
+void
+Http2ClientSession::add_to_keep_alive_queue()
+{
+ if (!is_active()) {
+ return;
+ }
+
+ if (connection_state.get_client_stream_count() != 0) {
+ return;
+ }
+
+ REMEMBER(NO_EVENT, this->recursion);
+ clear_session_active();
+
+ if (session_state == &Http2ClientSession::state_aborted) {
+ // mimicking VC_EVENT_INACTIVITY_TIMEOUT from
NetHandler::add_to_keep_alive_queue()
+ handleEvent(VC_EVENT_INACTIVITY_TIMEOUT);
+ return;
+ }
+
+ Http2SsnDebug("keep-alive queue - no streams");
+
+ UnixNetVConnection *vc = static_cast<UnixNetVConnection *>(_vc);
+ if (vc && vc->active_timeout_in == 0) {
+ // With heavy traffic ( - e.g. hitting proxy.config.net.max_connections_in
limit), calling add_to_keep_alive_queue() could
+ // destroy connections in the keep_alive_queue include this vc. In that
case, NetHandler raise VC_EVENT_INACTIVITY_TIMEOUT
+ // event to this ProxySession.
+ vc->add_to_keep_alive_queue();
+ }
+}
+
+bool
+Http2ClientSession::is_state_open() const
+{
+ return session_state == &Http2ClientSession::state_open;
+}
+
+bool
+Http2ClientSession::is_state_closed() const
+{
+ return session_state == &Http2ClientSession::state_closed;
+}
+
+bool
+Http2ClientSession::is_state_readable() const
+{
+ return session_state == &Http2ClientSession::state_open || session_state ==
&Http2ClientSession::state_graceful_shutdown;
+}
+
+bool
+Http2ClientSession::is_state_writable() const
+{
+ return session_state == &Http2ClientSession::state_open || session_state ==
&Http2ClientSession::state_graceful_shutdown ||
+ session_state == &Http2ClientSession::state_goaway;
+}
+
+/**
+ HTTP2_SESSION_EVENT_REENABLE handler for state_open &
state_graceful_shutdown
+ */
+int
+Http2ClientSession::_handle_ssn_reenable_event(int event, void *edata)
+{
+ ink_assert(session_state == &Http2ClientSession::state_open || session_state
== &Http2ClientSession::state_graceful_shutdown);
+ ink_assert(event == HTTP2_SESSION_EVENT_REENABLE);
+
+ if (edata == nullptr) {
+ return EVENT_DONE;
+ }
+
+ Event *e = static_cast<Event *>(edata);
+
+ // VIO will be reenableed in this handler
+ int res = (this->*session_handler)(VC_EVENT_READ_READY, static_cast<VIO
*>(e->cookie));
+
+ // Clear the event after calling session_handler to not reschedule REENABLE
in it
+ this->_reenable_event = nullptr;
+
+ return res;
+}
+
+/**
+ VC event handler for state_open & state_graceful_shutdown
+ */
+int
+Http2ClientSession::_handle_vc_event(int event, void *edata)
+{
+ ink_assert(session_state == &Http2ClientSession::state_open || session_state
== &Http2ClientSession::state_graceful_shutdown);
+
+ int res = EVENT_DONE;
+
+ switch (event) {
+ case VC_EVENT_READ_READY:
+ case VC_EVENT_READ_COMPLETE: {
+ ink_assert(edata != nullptr);
+ ink_assert(edata == read_vio);
+
+ res = (this->*session_handler)(event, edata);
+
+ break;
+ }
+ case VC_EVENT_WRITE_READY:
+ case VC_EVENT_WRITE_COMPLETE: {
+ ink_assert(edata != nullptr);
+ ink_assert(edata == write_vio);
+
+ this->connection_state.restart_streams();
+ if ((Thread::get_hrtime() >= this->_write_buffer_last_flush +
HRTIME_MSECONDS(this->_write_time_threshold))) {
+ this->flush();
+ }
+
+ break;
+ }
+ case VC_EVENT_ACTIVE_TIMEOUT:
+ case VC_EVENT_INACTIVITY_TIMEOUT: {
+ dying_event = event;
+
+ REMEMBER(event, recursion);
+ Http2SsnDebug("state_goaway by %s (%d)", get_vc_event_name(event), event);
+ _set_state_goaway(Http2ErrorCode::HTTP2_ERROR_INTERNAL_ERROR);
+
+ // signal timeout event to txns
+ connection_state.cleanup_streams(event);
+
+ break;
+ }
+ case VC_EVENT_ERROR:
+ case VC_EVENT_EOS: {
+ HTTP2_SET_SESSION_STATE(&Http2ClientSession::state_aborted);
+ Http2SsnDebug("state_aborted by %s (%d)", get_vc_event_name(event), event);
+
+ dying_event = event;
+
+ // No more read/write op
+ _vc->do_io_close();
+ _vc = nullptr;
+
Review comment:
I presume this is the immediate cause of the state_cache_open_write
crash. In the core dying_event is set to 104. But presumably we did not get
the HttpSM to stop directly as a result of the cleanup_streams() and the
cache_write_open event snuck in before we could get everything shutdown.
--
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]
