bneradt commented on code in PR #13834:
URL: https://github.com/apache/trafficserver/pull/13834#discussion_r4316414952
##########
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:
[P2] Start the thread you created rather than the shared list's last entry
In goaway-split, serve_socket runs in workers while the accept loop also
appends and starts entries in threads. If a worker appends its finisher here,
the accept loop then appends and starts another connection worker, and the
first worker resumes at threads[-1].start(), it tries to start that other
worker a second time. Its own finisher never starts, so shutdown can also fail
when joining it. I reproduced this interleaving with the unchanged run_server
function; it raises RuntimeError: cannot join thread before it is started. Keep
a local reference to each new Thread and start that object, both here and in
the accept loop at lines 309-310, so concurrent appends cannot change the
selected thread.
--
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]
