bneradt commented on PR #13911:
URL: https://github.com/apache/trafficserver/pull/13911#issuecomment-6422771114

   Thanks for the careful review, @bryancall. Addressed in 409060b34b:
   
   **Blocking: double close.** You're right: `chain_abort_all()` had already 
closed the transform, and nulling `t_info->vc` made `transform_cleanup()` close 
the hook chain a second time. This path now keeps `t_info->vc` the way Case 1 
does, so cleanup skips the already-closed chain.
   
   **Test**
   - Added `assert ats.is_running` after the 502.
   - The `HTTP/2 unknown case` check can fire now. I lowered 
`transaction_no_activity_timeout_in` to 5s, under curl's 10s limit, and moved 
the check ahead of the response assertions. With the `HttpSM.cc` change 
reverted, both HTTP/2 cases fail on that assertion with the warning logged once 
a second, and both HTTP/1.1 cases fail with an empty reply.
   - The UDS and `Curl.supports("http2")` skips now apply only to the HTTP/2 
cases. HTTP/1.1 uses the plain HTTP port, so it also runs in UDS mode.
   - Added the `http1.1`/`stall` case.
   
   **Cache read failure.** Agreed that 502 reads oddly there. The 502 is now 
set only when the aborted producer is the origin server, so a failed cache read 
feeding a transform gets the existing 500 from the transform error path.
   
   With the fix, all 4 cases pass over TCP. In UDS mode the 2 HTTP/1.1 cases 
pass and the 2 HTTP/2 cases skip. The other compress tests still pass.
   


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