Skip to content

Commit dd288da

Browse files
encukoumiss-islington
authored andcommitted
pythongh-126500: test_ssl: Don't stop ThreadedEchoServer on OSError in ConnectionHandler; rely on __exit__ (pythonGH-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]>
1 parent 9ecaee6 commit dd288da

File tree

1 file changed

+12
-5
lines changed

1 file changed

+12
-5
lines changed

Lib/test/test_ssl.py

+12-5
Original file line numberDiff line numberDiff line change
@@ -2312,7 +2312,6 @@ def wrap_conn(self):
23122312
# See also http://erickt.github.io/blog/2014/11/19/adventures-in-debugging-a-potential-osx-kernel-bug/
23132313
if e.errno != errno.EPROTOTYPE and sys.platform != "darwin":
23142314
self.running = False
2315-
self.server.stop()
23162315
self.close()
23172316
return False
23182317
else:
@@ -2449,10 +2448,6 @@ def run(self):
24492448
self.close()
24502449
self.running = False
24512450

2452-
# normally, we'd just stop here, but for the test
2453-
# harness, we want to stop the server
2454-
self.server.stop()
2455-
24562451
def __init__(self, certificate=None, ssl_version=None,
24572452
certreqs=None, cacerts=None,
24582453
chatty=True, connectionchatty=False, starttls_server=False,
@@ -2486,21 +2481,33 @@ def __init__(self, certificate=None, ssl_version=None,
24862481
self.conn_errors = []
24872482
threading.Thread.__init__(self)
24882483
self.daemon = True
2484+
self._in_context = False
24892485

24902486
def __enter__(self):
2487+
if self._in_context:
2488+
raise ValueError('Re-entering ThreadedEchoServer context')
2489+
self._in_context = True
24912490
self.start(threading.Event())
24922491
self.flag.wait()
24932492
return self
24942493

24952494
def __exit__(self, *args):
2495+
assert self._in_context
2496+
self._in_context = False
24962497
self.stop()
24972498
self.join()
24982499

24992500
def start(self, flag=None):
2501+
if not self._in_context:
2502+
raise ValueError(
2503+
'ThreadedEchoServer must be used as a context manager')
25002504
self.flag = flag
25012505
threading.Thread.start(self)
25022506

25032507
def run(self):
2508+
if not self._in_context:
2509+
raise ValueError(
2510+
'ThreadedEchoServer must be used as a context manager')
25042511
self.sock.settimeout(1.0)
25052512
self.sock.listen(5)
25062513
self.active = True

0 commit comments

Comments
 (0)