This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 75be8e1c8ca77b0b56a007fd762110ab39fd5b63 Author: Masakazu Kitajo <[email protected]> AuthorDate: Mon Apr 1 12:11:39 2024 -0600 Don't delete QUIC stream until both read and write complete (#11196) * Don't use reference for stream id * Don't delete QUIC stream until both read and write complete (cherry picked from commit b27449b1d23105d51621b4f4a865feaa66c248f2) --- include/iocore/net/quic/QUICStreamManager.h | 6 ++-- include/iocore/net/quic/QUICStreamVCAdapter.h | 4 +++ include/proxy/http3/Http3App.h | 2 ++ src/iocore/net/quic/QUICStreamManager.cc | 6 ++-- src/iocore/net/quic/QUICStreamVCAdapter.cc | 12 +++++++ src/proxy/http3/Http3App.cc | 48 +++++++++++++++++++-------- 6 files changed, 59 insertions(+), 19 deletions(-) diff --git a/include/iocore/net/quic/QUICStreamManager.h b/include/iocore/net/quic/QUICStreamManager.h index fdf114cf4f..ff9fdef690 100644 --- a/include/iocore/net/quic/QUICStreamManager.h +++ b/include/iocore/net/quic/QUICStreamManager.h @@ -47,9 +47,9 @@ public: QUICStream *find_stream(QUICStreamId stream_id); QUICConnectionErrorUPtr create_stream(QUICStreamId stream_id); - QUICConnectionErrorUPtr create_uni_stream(QUICStreamId &new_stream_id); - QUICConnectionErrorUPtr create_bidi_stream(QUICStreamId &new_stream_id); - QUICConnectionErrorUPtr delete_stream(QUICStreamId &new_stream_id); + QUICConnectionErrorUPtr create_uni_stream(QUICStreamId new_stream_id); + QUICConnectionErrorUPtr create_bidi_stream(QUICStreamId new_stream_id); + QUICConnectionErrorUPtr delete_stream(QUICStreamId new_stream_id); void reset_stream(QUICStreamId stream_id, QUICStreamErrorUPtr error); void set_default_application(QUICApplication *app); diff --git a/include/iocore/net/quic/QUICStreamVCAdapter.h b/include/iocore/net/quic/QUICStreamVCAdapter.h index b787896548..4e7a1e6afd 100644 --- a/include/iocore/net/quic/QUICStreamVCAdapter.h +++ b/include/iocore/net/quic/QUICStreamVCAdapter.h @@ -50,6 +50,10 @@ public: void do_io_shutdown(ShutdownHowTo_t howto) override; void reenable(VIO *vio) override; + // Helpers to check VIO states + bool is_readable(); + bool is_writable(); + void clear_read_ready_event(Event *e); void clear_read_complete_event(Event *e); void clear_write_ready_event(Event *e); diff --git a/include/proxy/http3/Http3App.h b/include/proxy/http3/Http3App.h index 26e6d51985..d9b59434e5 100644 --- a/include/proxy/http3/Http3App.h +++ b/include/proxy/http3/Http3App.h @@ -69,10 +69,12 @@ protected: private: void _handle_uni_stream_on_read_ready(int event, VIO *vio); + void _handle_uni_stream_on_read_complete(int event, VIO *vio); void _handle_uni_stream_on_write_ready(int event, VIO *vio); void _handle_uni_stream_on_write_complete(int event, VIO *vio); void _handle_uni_stream_on_eos(int event, VIO *vio); void _handle_bidi_stream_on_read_ready(int event, VIO *vio); + void _handle_bidi_stream_on_read_complete(int event, VIO *vio); void _handle_bidi_stream_on_write_ready(int event, VIO *vio); void _handle_bidi_stream_on_write_complete(int event, VIO *vio); void _handle_bidi_stream_on_eos(int event, VIO *vio); diff --git a/src/iocore/net/quic/QUICStreamManager.cc b/src/iocore/net/quic/QUICStreamManager.cc index e524795005..eb241c23a8 100644 --- a/src/iocore/net/quic/QUICStreamManager.cc +++ b/src/iocore/net/quic/QUICStreamManager.cc @@ -101,19 +101,19 @@ QUICStreamManager::create_stream(QUICStreamId stream_id) } QUICConnectionErrorUPtr -QUICStreamManager::create_uni_stream(QUICStreamId &new_stream_id) +QUICStreamManager::create_uni_stream(QUICStreamId new_stream_id) { return nullptr; } QUICConnectionErrorUPtr -QUICStreamManager::create_bidi_stream(QUICStreamId &new_stream_id) +QUICStreamManager::create_bidi_stream(QUICStreamId new_stream_id) { return nullptr; } QUICConnectionErrorUPtr -QUICStreamManager::delete_stream(QUICStreamId &stream_id) +QUICStreamManager::delete_stream(QUICStreamId stream_id) { QUICStream *stream = static_cast<QUICStream *>(this->find_stream(stream_id)); diff --git a/src/iocore/net/quic/QUICStreamVCAdapter.cc b/src/iocore/net/quic/QUICStreamVCAdapter.cc index e742572f7d..6abf119627 100644 --- a/src/iocore/net/quic/QUICStreamVCAdapter.cc +++ b/src/iocore/net/quic/QUICStreamVCAdapter.cc @@ -328,6 +328,18 @@ QUICStreamVCAdapter::reenable(VIO *vio) // until the application consume data. } +bool +QUICStreamVCAdapter::is_readable() +{ + return this->stream().direction() != QUICStreamDirection::SEND && _read_vio.nbytes == _read_vio.ndone; +} + +bool +QUICStreamVCAdapter::is_writable() +{ + return this->stream().direction() != QUICStreamDirection::RECEIVE && _write_vio.nbytes != -1; +} + int QUICStreamVCAdapter::state_stream_open(int event, void *data) { diff --git a/src/proxy/http3/Http3App.cc b/src/proxy/http3/Http3App.cc index 97ec43e833..f324144300 100644 --- a/src/proxy/http3/Http3App.cc +++ b/src/proxy/http3/Http3App.cc @@ -153,11 +153,10 @@ Http3App::main_event_handler(int event, Event *data) break; case VC_EVENT_READ_COMPLETE: adapter->clear_read_complete_event(data); - // Calling read_ready handlers because there's no need to do different things if (is_bidirectional) { - this->_handle_bidi_stream_on_read_ready(event, vio); + this->_handle_bidi_stream_on_read_complete(event, vio); } else { - this->_handle_uni_stream_on_read_ready(event, vio); + this->_handle_uni_stream_on_read_complete(event, vio); } break; case VC_EVENT_WRITE_READY: @@ -282,6 +281,17 @@ Http3App::_handle_uni_stream_on_read_ready(int /* event */, VIO *vio) } } +void +Http3App::_handle_uni_stream_on_read_complete(int event, VIO *vio) +{ + this->_handle_uni_stream_on_read_ready(event, vio); + + QUICStreamVCAdapter *adapter = static_cast<QUICStreamVCAdapter *>(vio->vc_server); + if (!adapter->is_writable()) { + this->_qc->stream_manager()->delete_stream(adapter->stream().id()); + } +} + void Http3App::_handle_bidi_stream_on_read_ready(int event, VIO *vio) { @@ -304,6 +314,17 @@ Http3App::_handle_bidi_stream_on_read_ready(int event, VIO *vio) } } +void +Http3App::_handle_bidi_stream_on_read_complete(int event, VIO *vio) +{ + this->_handle_bidi_stream_on_read_ready(event, vio); + + QUICStreamVCAdapter *adapter = static_cast<QUICStreamVCAdapter *>(vio->vc_server); + if (!adapter->is_writable()) { + this->_qc->stream_manager()->delete_stream(adapter->stream().id()); + } +} + void Http3App::_handle_uni_stream_on_write_ready(int /* event */, VIO *vio) { @@ -343,9 +364,14 @@ Http3App::_handle_uni_stream_on_write_ready(int /* event */, VIO *vio) } void -Http3App::_handle_uni_stream_on_write_complete(int /* event */, VIO *vio) +Http3App::_handle_uni_stream_on_write_complete(int event, VIO *vio) { - // QUICStreamVCAdapter *adapter = static_cast<QUICStreamVCAdapter *>(vio->vc_server); + this->_handle_uni_stream_on_write_ready(event, vio); + + QUICStreamVCAdapter *adapter = static_cast<QUICStreamVCAdapter *>(vio->vc_server); + if (!adapter->is_readable()) { + this->_qc->stream_manager()->delete_stream(adapter->stream().id()); + } } void @@ -431,14 +457,10 @@ Http3App::_handle_bidi_stream_on_write_ready(int event, VIO *vio) void Http3App::_handle_bidi_stream_on_write_complete(int event, VIO *vio) { - QUICStreamVCAdapter *adapter = static_cast<QUICStreamVCAdapter *>(vio->vc_server); + this->_handle_bidi_stream_on_write_ready(event, vio); - QUICStreamId stream_id = adapter->stream().id(); - Http3Transaction *txn = static_cast<Http3Transaction *>(this->_ssn->get_transaction(stream_id)); - if (txn != nullptr) { - SCOPED_MUTEX_LOCK(lock, txn->mutex, this_ethread()); - txn->handleEvent(event); + QUICStreamVCAdapter *adapter = static_cast<QUICStreamVCAdapter *>(vio->vc_server); + if (!adapter->is_readable()) { + this->_qc->stream_manager()->delete_stream(adapter->stream().id()); } - // FIXME There may be data to read - this->_qc->stream_manager()->delete_stream(stream_id); }
