Skip to content

Commit 2bd953b

Browse files
committed
Fix a thread leak.
Implementing correct shutdown() would involve copying too much code, so switch to the more traditional way of indicating shutdown to a thread reading from a Queue. Signed-off-by: Itamar Turner-Trauring <[email protected]>
1 parent 22bb430 commit 2bd953b

File tree

3 files changed

+59
-36
lines changed

3 files changed

+59
-36
lines changed

cheroot/server.py

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -100,37 +100,6 @@
100100
'get_ssl_adapter_class',
101101
)
102102

103-
104-
if sys.version_info[:2] >= (3, 13):
105-
from queue import (
106-
Queue as QueueWithShutdown,
107-
ShutDown as QueueShutDown,
108-
)
109-
else:
110-
111-
class QueueShutDown(Exception):
112-
"""Queue has been shut down."""
113-
114-
class QueueWithShutdown(queue.Queue):
115-
"""Add shutdown() similar to Python 3.13+ Queue."""
116-
117-
_queue_shut_down: bool = False
118-
119-
def shutdown(self, immediate=False):
120-
if immediate:
121-
while True:
122-
try:
123-
self.get_nowait()
124-
except queue.Empty:
125-
break
126-
self._queue_shut_down = True
127-
128-
def get(self, *args, **kwargs):
129-
if self._queue_shut_down:
130-
raise QueueShutDown
131-
return super().get(*args, **kwargs)
132-
133-
134103
IS_WINDOWS = platform.system() == 'Windows'
135104
"""Flag indicating whether the app is running under Windows."""
136105

@@ -1691,7 +1660,7 @@ def __init__( # pylint: disable=too-many-positional-arguments
16911660
self.reuse_port = reuse_port
16921661
self.clear_stats()
16931662

1694-
self._unservicable_conns = QueueWithShutdown()
1663+
self._unservicable_conns = queue.Queue()
16951664

16961665
def clear_stats(self):
16971666
"""Reset server stat counters.."""
@@ -1904,9 +1873,8 @@ def prepare(self): # noqa: C901 # FIXME
19041873
def _serve_unservicable(self):
19051874
"""Serve connections we can't handle a 503."""
19061875
while self.ready:
1907-
try:
1908-
conn = self._unservicable_conns.get()
1909-
except QueueShutDown:
1876+
conn = self._unservicable_conns.get()
1877+
if conn is None:
19101878
return
19111879
request = HTTPRequest(self, conn)
19121880
try:
@@ -2269,7 +2237,7 @@ def stop(self): # noqa: C901 # FIXME
22692237

22702238
# This tells the thread that handles unservicable connections to shut
22712239
# down:
2272-
self._unservicable_conns.shutdown(immediate=True)
2240+
self._unservicable_conns.put(None)
22732241

22742242
if self._start_time is not None:
22752243
self._run_time += time.time() - self._start_time

cheroot/test/test_server.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
"""Tests for the HTTP server."""
22

33
import os
4+
import pathlib
45
import queue
56
import socket
7+
import subprocess
68
import sys
79
import tempfile
810
import threading
@@ -611,3 +613,15 @@ def test_overload_results_in_suitable_http_error(request):
611613

612614
response = requests.get(f'http://{localhost}:{port}', timeout=20)
613615
assert response.status_code == HTTPStatus.SERVICE_UNAVAILABLE
616+
617+
618+
def test_overload_thread_does_not_leak():
619+
"""On shutdown the overload thread exits.
620+
621+
This is a test for issue #769.
622+
"""
623+
path = pathlib.Path(__file__).parent / 'threadleakcheck.py'
624+
process = subprocess.run([sys.executable, path], check=False)
625+
# We use exit code 23 to indicate success, so the test doesn't acidentally
626+
# pass:
627+
assert process.returncode == 23

cheroot/test/threadleakcheck.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
"""
2+
Make sure threads don't leak.
3+
4+
Run in an isolated subprocess by test_server.py, to ensure parallelism of any
5+
sort don't cause problems.
6+
"""
7+
8+
import os
9+
import sys
10+
import threading
11+
import time
12+
13+
from cheroot.server import Gateway, HTTPServer
14+
from cheroot.testing import (
15+
ANY_INTERFACE_IPV4,
16+
EPHEMERAL_PORT,
17+
)
18+
19+
20+
def check_for_leaks():
21+
"""Exit with code 23 if no threads were leaked."""
22+
before_serv = threading.active_count()
23+
for _ in range(5):
24+
httpserver = HTTPServer(
25+
bind_addr=(ANY_INTERFACE_IPV4, EPHEMERAL_PORT),
26+
gateway=Gateway,
27+
)
28+
with httpserver._run_in_thread():
29+
time.sleep(0.2)
30+
31+
leaked_threads = threading.active_count() - before_serv
32+
if leaked_threads == 0:
33+
os._exit(23)
34+
else:
35+
# We leaked a thread:
36+
print('Number of leaked threads:', leaked_threads, file=sys.stderr)
37+
os._exit(7)
38+
39+
40+
if __name__ == '__main__':
41+
check_for_leaks()

0 commit comments

Comments
 (0)