Skip to content

Commit 00fe666

Browse files
gh-141044: Fix ASan leak with small threading.stack_size()
AddressSanitizer instrumentation consumes extra C stack, so the old minimum left no working space above the soft recursion limit. threading.Thread bootstrap then leaked thread objects. Require 6 stack margins under ASan, matching TSan. Cover the unjoined original repro and confirm the new minimum does not leak. Move the NEWS blurb to Library, matching gh-143191.
1 parent 23180c5 commit 00fe666

4 files changed

Lines changed: 153 additions & 1 deletion

File tree

‎Include/internal/pycore_pythonrun.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,12 @@ extern PyObject* _PyRun_SimpleString(
6262
# define _PyOS_STACK_MARGIN_SHIFT (_PyOS_LOG2_STACK_MARGIN + 2)
6363
#endif
6464

65-
#ifdef _Py_THREAD_SANITIZER
65+
#if defined(_Py_THREAD_SANITIZER) || defined(_Py_ADDRESS_SANITIZER)
66+
/* Sanitizer builds need more than the default 3 margins:
67+
* - TSan: tstate_set_stack() only uses half the stack.
68+
* - ASan (gh-141044): instrumentation consumes extra C stack, so 3
69+
* margins leave threading.Thread bootstrap with no working space
70+
* above the soft recursion limit and leak thread objects. */
6671
# define _PyOS_MIN_STACK_SIZE (_PyOS_STACK_MARGIN_BYTES * 6)
6772
#else
6873
# define _PyOS_MIN_STACK_SIZE (_PyOS_STACK_MARGIN_BYTES * 3)

‎Lib/test/test_thread.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,12 @@ def test_stack_size(self):
8484
# size must be positive
8585
thread.stack_size(-4096)
8686

87+
if support.check_sanitizer(address=True, function=False):
88+
# gh-141044: 127 KiB used to be accepted but leaked under ASan
89+
with self.assertRaises(ValueError):
90+
thread.stack_size(127 * 1024)
91+
self.assertEqual(thread.stack_size(), 0)
92+
8793
@unittest.skipIf(os.name not in ("nt", "posix"), 'test meant for nt and posix')
8894
def test_nt_and_posix_stack_size(self):
8995
try:

‎Lib/test/test_threading.py‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,140 @@ def test_various_ops_large_stack(self):
269269
self.test_various_ops()
270270
threading.stack_size(0)
271271

272+
def test_stack_size_no_leak(self):
273+
# gh-141044: a custom thread stack size used to leak Thread objects
274+
# when the size passed _thread.stack_size() but was too small for
275+
# threading.Thread bootstrap under AddressSanitizer. The reported
276+
# repro used 127 KiB and did not join the thread.
277+
try:
278+
threading.stack_size(262144)
279+
except _thread.error:
280+
self.skipTest(
281+
'platform does not support changing thread stack size')
282+
threading.stack_size(0)
283+
284+
def run_script(script):
285+
_, _, err = assert_python_ok(
286+
"-c", textwrap.dedent(script),
287+
ASAN_OPTIONS="detect_leaks=1:halt_on_error=1")
288+
err_s = err.decode("utf-8", "replace")
289+
self.assertNotIn("LeakSanitizer", err_s, err_s)
290+
291+
min_stack_helper = """
292+
import os
293+
import threading
294+
import _thread
295+
296+
def min_stack_size():
297+
page = os.sysconf("SC_PAGESIZE") if hasattr(os, "sysconf") else 4096
298+
size = page
299+
while size <= 4 * 1024 * 1024:
300+
try:
301+
threading.stack_size(size)
302+
except ValueError:
303+
size += page
304+
continue
305+
except _thread.error:
306+
return None
307+
return size
308+
return None
309+
"""
310+
311+
# Original reproducer: start, do not join. 127 KiB is rejected on
312+
# ASan builds; if a build still accepts it, the thread must not leak.
313+
run_script("""
314+
import threading
315+
try:
316+
threading.stack_size(127 * 1024)
317+
except ValueError:
318+
raise SystemExit(0)
319+
def worker():
320+
pass
321+
t = threading.Thread(target=worker, name="worker-thread")
322+
t.start()
323+
threading.stack_size(0)
324+
""")
325+
326+
# Smallest accepted size, unjoined. Wait until the worker finishes
327+
# without join(); process shutdown also joins. LSan plus (on debug
328+
# builds) gettotalrefcount() must stay clean at this new minimum.
329+
run_script(min_stack_helper + """
330+
import gc
331+
import sys
332+
import time
333+
334+
def worker():
335+
pass
336+
size = min_stack_size()
337+
if size is None:
338+
raise SystemExit(0)
339+
340+
def wait_unjoined(threads, timeout=30):
341+
deadline = time.monotonic() + timeout
342+
for t in threads:
343+
while t.is_alive():
344+
if time.monotonic() > deadline:
345+
raise SystemExit("unjoined worker did not finish")
346+
time.sleep(0.001)
347+
348+
if hasattr(sys, "gettotalrefcount"):
349+
gc.collect()
350+
gc.collect()
351+
start = sys.gettotalrefcount()
352+
threads = []
353+
for _ in range(8):
354+
t = threading.Thread(target=worker)
355+
t.start()
356+
threads.append(t)
357+
wait_unjoined(threads)
358+
del threads
359+
threading.stack_size(0)
360+
gc.collect()
361+
gc.collect()
362+
delta = sys.gettotalrefcount() - start
363+
if delta > 50:
364+
raise SystemExit(f"refcount leak: {delta}")
365+
else:
366+
t = threading.Thread(target=worker, name="min-stack-worker")
367+
t.start()
368+
threading.stack_size(0)
369+
""")
370+
371+
# Joined threads at the minimum must not leak references.
372+
run_script(min_stack_helper + """
373+
import gc
374+
import sys
375+
376+
def worker():
377+
pass
378+
size = min_stack_size()
379+
if size is None:
380+
raise SystemExit(0)
381+
for _ in range(3):
382+
t = threading.Thread(target=worker)
383+
t.start()
384+
t.join()
385+
if hasattr(sys, "gettotalrefcount"):
386+
gc.collect()
387+
gc.collect()
388+
start = sys.gettotalrefcount()
389+
for _ in range(8):
390+
t = threading.Thread(target=worker)
391+
t.start()
392+
t.join()
393+
threading.stack_size(0)
394+
gc.collect()
395+
gc.collect()
396+
delta = sys.gettotalrefcount() - start
397+
if delta > 50:
398+
raise SystemExit(f"refcount leak: {delta}")
399+
else:
400+
t = threading.Thread(target=worker)
401+
t.start()
402+
t.join()
403+
threading.stack_size(0)
404+
""")
405+
272406
def test_foreign_thread(self):
273407
# Check that a "foreign" thread can use the threading module.
274408
dummy_thread = None
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Fix a reference leak when creating a :class:`threading.Thread` after
2+
setting a small custom stack size with :func:`threading.stack_size` in
3+
AddressSanitizer builds. ASan instrumentation needs extra C stack, so
4+
:func:`_thread.stack_size` now requires the same 6 stack margins used
5+
with ThreadSanitizer. Sizes that previously appeared to work under ASan
6+
(for example 127 KiB) now raise :exc:`ValueError`. Non-ASan builds are
7+
unchanged. Patch by Kailash Nelson.

0 commit comments

Comments
 (0)