Copilot commented on code in PR #13186:
URL: https://github.com/apache/trafficserver/pull/13186#discussion_r3319648819
##########
src/proxy/http3/Http3StreamDataVIOAdaptor.cc:
##########
@@ -53,17 +57,17 @@
Http3StreamDataVIOAdaptor::handle_frame(std::shared_ptr<const Http3Frame> frame,
void
Http3StreamDataVIOAdaptor::finalize()
{
- SCOPED_MUTEX_LOCK(lock, this->_sink_vio->mutex, this_ethread());
- MIOBuffer *writer = this->_sink_vio->get_writer();
- IOBufferReader *reader = this->_buffer->alloc_reader();
- IOBufferBlock *block;
- while (reader->read_avail() > 0 && (block = reader->get_current_block()) !=
nullptr) {
- writer->append_block(block);
- reader->consume(block->size());
+ if (this->_finalized) {
+ return;
}
- this->_buffer->dealloc_reader(reader);
- this->_sink_vio->nbytes = this->_total_data_length;
+ SCOPED_MUTEX_LOCK(lock, this->_sink_vio->mutex, this_ethread());
+ MIOBuffer *writer = this->_sink_vio->get_writer();
+ int64_t delivered = writer->write(this->_reader,
this->_reader->read_avail());
+ this->_reader->consume(delivered);
+ this->_sink_vio->nbytes = this->_total_data_length;
+ this->_sink_vio->ndone += delivered;
+ this->_finalized = true;
Review Comment:
`Http3StreamDataVIOAdaptor::finalize()` updates the sink `VIO`'s `ndone` and
sets `nbytes` to `_total_data_length` (DATA payload bytes only). This breaks
the usual VIO contract where `ndone` tracks bytes consumed by the reader, and
it can make `ntodo()` negative once the consumer has already consumed header
bytes written by `Http3HeaderVIOAdaptor` (same sink VIO). It also risks
signaling `VC_EVENT_READ_COMPLETE` prematurely/incorrectly.
A safer approach is to avoid touching `ndone` here, and set `nbytes` based
on the total number of bytes that will be available on this VIO (including any
already-written header bytes), or otherwise ensure the completion condition is
derived from the actual end-of-stream rather than manipulating `ndone`.
--
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]
