This is an automated email from the ASF dual-hosted git repository.
maskit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new b27449b1d2 Don't delete QUIC stream until both read and write complete
(#11196)
b27449b1d2 is described below
commit b27449b1d23105d51621b4f4a865feaa66c248f2
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
---
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);
}
