Copilot commented on code in PR #13410:
URL: https://github.com/apache/trafficserver/pull/13410#discussion_r3708927405


##########
src/proxy/http/HttpTunnel.cc:
##########
@@ -1109,9 +1109,19 @@ HttpTunnel::producer_run(HttpTunnelProducer *p)
     }
 
     if (c_write == 0) {
-      // Nothing to do, call back the cleanup handlers
-      c->write_vio = nullptr;
-      consumer_handler(VC_EVENT_WRITE_COMPLETE, c);
+      // Cache writes need a VIO even when the body is empty so that closing 
the
+      // cache VC commits the response metadata instead of aborting the write.
+      if (c->vc_type == HttpTunnelType_t::CACHE_WRITE) {
+        c->write_vio = c->vc->do_io_write(this, 0, c->buffer_reader);
+        if (c->write_vio == nullptr) {
+          consumer_handler(VC_EVENT_ERROR, c);
+        } else if (c->write_vio->ntodo() == 0 && c->alive) {
+          consumer_handler(VC_EVENT_WRITE_COMPLETE, c);
+        }

Review Comment:
   In the CACHE_WRITE + `c_write == 0` path, calling 
`consumer_handler(VC_EVENT_WRITE_COMPLETE, c)` is likely to fire twice: 
`CacheVC::openWriteMain()` will also call back the continuation with 
`VC_EVENT_WRITE_COMPLETE` when `vio.ntodo() <= 0`. If the tunnel marks the 
consumer dead here, the later cache callback can hit `ink_assert(c->alive == 
true)` in `HttpTunnel::consumer_handler()`.
   
   For cache writes, let the cache VC deliver `VC_EVENT_WRITE_COMPLETE` (or 
`VC_EVENT_ERROR`) via the normal VIO callback instead of calling 
`consumer_handler()` directly.



-- 
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]

Reply via email to