Skip to content

Commit

Permalink
[3.13] gh-126500: test_ssl: Don't stop ThreadedEchoServer on OSError …
Browse files Browse the repository at this point in the history
…in ConnectionHandler; rely on __exit__ (GH-126503) (GH-126571)

gh-126500: test_ssl: Don't stop ThreadedEchoServer on OSError in ConnectionHandler; rely on __exit__ (GH-126503)

If `read()` in the ConnectionHandler thread raises `OSError` (except `ConnectionError`),
the ConnectionHandler shuts down the entire ThreadedEchoServer,
preventing further connections.
It also does that for `EPROTOTYPE` in `wrap_conn`.

As far as I can see, this is done to avoid the server thread getting stuck,
forgotten, in its accept loop. However, since 2011 (5b95eb9)
the server is used as a context manager, and its `__exit__` does `stop()` and `join()`.
(I'm not sure if we *always* used `with` since that commit, but currently we do.)

Make sure that the context manager *is* used, and remove the `server.stop()`
calls from ConnectionHandler.
(cherry picked from commit c9cda16)

Co-authored-by: Petr Viktorin <[email protected]>
  • Loading branch information
miss-islington and encukou authored Nov 11, 2024
1 parent 1e4b9c7 commit fc10908
Showing 1 changed file with 12 additions and 5 deletions.
17 changes: 12 additions & 5 deletions Lib/test/test_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -2312,7 +2312,6 @@ def wrap_conn(self):
# See also http://erickt.github.io/blog/2014/11/19/adventures-in-debugging-a-potential-osx-kernel-bug/
if e.errno != errno.EPROTOTYPE and sys.platform != "darwin":
self.running = False
self.server.stop()
self.close()
return False
else:
Expand Down Expand Up @@ -2449,10 +2448,6 @@ def run(self):
self.close()
self.running = False

# normally, we'd just stop here, but for the test
# harness, we want to stop the server
self.server.stop()

def __init__(self, certificate=None, ssl_version=None,
certreqs=None, cacerts=None,
chatty=True, connectionchatty=False, starttls_server=False,
Expand Down Expand Up @@ -2486,21 +2481,33 @@ def __init__(self, certificate=None, ssl_version=None,
self.conn_errors = []
threading.Thread.__init__(self)
self.daemon = True
self._in_context = False

def __enter__(self):
if self._in_context:
raise ValueError('Re-entering ThreadedEchoServer context')
self._in_context = True
self.start(threading.Event())
self.flag.wait()
return self

def __exit__(self, *args):
assert self._in_context
self._in_context = False
self.stop()
self.join()

def start(self, flag=None):
if not self._in_context:
raise ValueError(
'ThreadedEchoServer must be used as a context manager')
self.flag = flag
threading.Thread.start(self)

def run(self):
if not self._in_context:
raise ValueError(
'ThreadedEchoServer must be used as a context manager')
self.sock.settimeout(1.0)
self.sock.listen(5)
self.active = True
Expand Down

0 comments on commit fc10908

Please sign in to comment.