Copilot commented on code in PR #13186:
URL: https://github.com/apache/trafficserver/pull/13186#discussion_r3329236059
##########
src/proxy/http3/Http3Session.cc:
##########
@@ -162,9 +162,16 @@ HQSession::main_event_handler(int event, void *edata)
case VC_EVENT_ERROR:
case VC_EVENT_EOS:
this->do_io_close();
- for (HQTransaction *t = this->_transaction_list.head; t; t =
static_cast<HQTransaction *>(t->link.next)) {
+ while (true) {
+ HQTransaction *t = this->_transaction_list.head;
+ if (t == nullptr) {
+ break;
+ }
SCOPED_MUTEX_LOCK(lock, t->mutex, this_ethread());
t->handleEvent(event, edata);
+ if (this->_transaction_list.head == t) {
+ break;
+ }
}
Review Comment:
On session teardown (EOS/ERROR/timeouts), this loop can exit after handling
only the current head transaction if that transaction does not remove itself
from `_transaction_list`. In that case, remaining transactions will never
receive the close event, and `HQSession`'s destructor assertion
(`_transaction_list.head == nullptr`) can be violated (leak / abort). Use an
iteration pattern that is safe if `handleEvent()` deletes the current
transaction, but still walks the entire list.
##########
src/proxy/http3/Http3App.cc:
##########
@@ -131,6 +131,9 @@ Http3App::on_stream_open(QUICStream &stream)
void
Http3App::on_stream_close(QUICStream &stream)
{
+ if (auto *txn = this->_ssn->get_transaction(stream.id()); txn != nullptr) {
+ txn->stream_closed();
+ }
this->_streams.erase(stream.id());
}
Review Comment:
`on_stream_close()` calls `txn->stream_closed()` without taking
`txn->mutex`, but `Http3Transaction` state handlers and other callers
consistently guard transaction state with this mutex. `stream_closed()` mutates
transaction state (e.g., `_stream_closed`, `_closed`, VIO fields) and may run
concurrently with an in-flight handler, leading to data races and inconsistent
teardown. Take the transaction mutex before invoking `stream_closed()`.
--
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]
