bryancall commented on code in PR #13834:
URL: https://github.com/apache/trafficserver/pull/13834#discussion_r4265051272
##########
tests/gold_tests/h2/safe_retry_origin.py:
##########
@@ -122,6 +122,26 @@ def _wait_for_close(self, tls_socket: ssl.SSLSocket) ->
None:
print("drained_session_closed_by_peer", flush=True)
return
+ def finish_connection(self, tls_socket: ssl.SSLSocket) -> None:
+ """Half-close and read until ATS closes, so that closing cannot reset
the connection.
+
+ Closing a socket that still holds unread input, such as a SETTINGS ACK
+ that ATS sent after the last read, makes the kernel reset the
connection,
+ which can discard what this origin just wrote, such as the retried
response.
+ """
+ try:
+ tls_socket.shutdown(socket.SHUT_WR)
+ except OSError:
+ return
+ while not self.lifecycle.stopped:
+ try:
+ if not tls_socket.recv(65535):
+ return
+ except TimeoutError:
+ continue
+ except OSError:
+ return
Review Comment:
Won't fix. `ssl.SSLZeroReturnError`, `ssl.SSLEOFError` and every other
`ssl.SSLError` subclass `OSError`, so the `except OSError` already catches
them. `SSLSocket.shutdown()` also drops the SSL object before it half-closes,
so the reads after it are plain socket reads.
##########
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,)))
Review Comment:
Won't fix. `run_server` joins every thread in `threads` before it returns,
finishers included. Each one reads with a 0.2 s timeout and checks
`lifecycle.stopped` on every pass, so it exits within 0.2 s of the stop file
and cannot hold up shutdown.
--
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]
