bryancall commented on code in PR #13834:
URL: https://github.com/apache/trafficserver/pull/13834#discussion_r4316653203
##########
tests/gold_tests/h2/safe_retry_origin.py:
##########
@@ -253,19 +278,25 @@ def serve_socket(plain_socket: socket.socket, listener:
socket.socket | None) ->
# Ignore readiness probes and peers that never finish TLS.
plain_socket.close()
return
- with tls_socket:
+ try:
if tls_socket.selected_alpn_protocol() != "h2":
raise RuntimeError("ATS did not negotiate HTTP/2 with the
origin")
tls_socket.settimeout(0.2)
origin.serve_connection(tls_socket, listener)
+ except BaseException:
+ tls_socket.close()
+ raise
+ # Finish in the background: ATS can keep a finished connection open
+ # while it opens the next one, which this origin must accept.
+ threads.append(threading.Thread(target=finish_socket,
args=(tls_socket,)))
+ threads[-1].start()
Review Comment:
Good catch, fixed in 81dc033e4a. Both places now create the thread, start it
through a local reference, and only then append it to `threads`, so a
concurrent append cannot change which thread gets started, and the join loop
never sees a thread that has not started.
To check it, I paused the worker just before its start line so the accept
loop could append and start another worker in that gap. The old code failed 50
of 50 runs with `RuntimeError: cannot join thread before it is started`, and
the new code passed 50 of 50. The full `h2_to_origin_safe_retry` test also
passed 64 of 64 runs in the Fedora 44 CI image under CPU load.
--
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]
