From c645ae5ab3375891d32097a7befc8303cf502f90 Mon Sep 17 00:00:00 2001 From: James David Clarke Date: Sun, 24 May 2026 09:22:51 +0100 Subject: [PATCH 1/6] Fix _thread panic when weakref callback fires during cleanup - Drop the target function inside an inner scope so its Python refs are released before cleanup_current_thread_frames clears CURRENT_THREAD_SLOT - Avoids panic in push_thread_frame when a weakref callback runs as `func` drops at end-of-function --- crates/vm/src/stdlib/_thread.rs | 55 +++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/crates/vm/src/stdlib/_thread.rs b/crates/vm/src/stdlib/_thread.rs index b6fbf146a96..11d6d45dad0 100644 --- a/crates/vm/src/stdlib/_thread.rs +++ b/crates/vm/src/stdlib/_thread.rs @@ -530,15 +530,23 @@ pub(crate) mod _thread { // Increment thread count when thread actually starts executing vm.state.thread_count.fetch_add(1); - match func.invoke(args, vm) { - Ok(_obj) => {} - Err(e) if e.fast_isinstance(vm.ctx.exceptions.system_exit) => {} - Err(exc) => { - vm.run_unraisable( - exc, - Some("Exception ignored in thread started by".to_owned()), - func.into(), - ); + // Inner scope: drop `func` (and its Python refs) before the thread + // slot is torn down below. Otherwise the parameter `func` would drop + // at end-of-function, after cleanup_current_thread_frames has cleared + // CURRENT_THREAD_SLOT, and a weakref callback fired during that drop + // would panic in push_thread_frame. + { + let func = func; + match func.invoke(args, vm) { + Ok(_obj) => {} + Err(e) if e.fast_isinstance(vm.ctx.exceptions.system_exit) => {} + Err(exc) => { + vm.run_unraisable( + exc, + Some("Exception ignored in thread started by".to_owned()), + func.into(), + ); + } } } for lock in SENTINELS.take() { @@ -1663,16 +1671,25 @@ pub(crate) mod _thread { // Increment thread count when thread actually starts executing vm_state.thread_count.fetch_add(1); - // Run the function - match func.invoke((), vm) { - Ok(_) => {} - Err(e) if e.fast_isinstance(vm.ctx.exceptions.system_exit) => {} - Err(exc) => { - vm.run_unraisable( - exc, - Some("Exception ignored in thread started by".to_owned()), - func.into(), - ); + // Inner scope: drop `func` (and its Python refs) before the + // outer scopeguard::defer tears down the thread slot. As a + // `move` closure capture, `func` would otherwise drop after + // all locals (including the scopeguard `_guard`), and a + // weakref callback fired during that drop would panic in + // push_thread_frame. + { + let func = func; + // Run the function + match func.invoke((), vm) { + Ok(_) => {} + Err(e) if e.fast_isinstance(vm.ctx.exceptions.system_exit) => {} + Err(exc) => { + vm.run_unraisable( + exc, + Some("Exception ignored in thread started by".to_owned()), + func.into(), + ); + } } } })) From e07be1beb67f4c2553b309166c17c420bceee3b6 Mon Sep 17 00:00:00 2001 From: James David Clarke Date: Sun, 24 May 2026 09:42:55 +0100 Subject: [PATCH 2/6] Address review feedback on PR #7965 - Rewrite both match blocks as `if let Err(exc) && !is(SystemExit)` per @ShaharNaveh - Add extra_tests/snippets/test_threading_teardown.py regression test from issue #7813 Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/vm/src/stdlib/_thread.rs | 36 +++++++++---------- .../snippets/test_threading_teardown.py | 9 +++++ 2 files changed, 25 insertions(+), 20 deletions(-) create mode 100644 extra_tests/snippets/test_threading_teardown.py diff --git a/crates/vm/src/stdlib/_thread.rs b/crates/vm/src/stdlib/_thread.rs index 11d6d45dad0..0af8d7add38 100644 --- a/crates/vm/src/stdlib/_thread.rs +++ b/crates/vm/src/stdlib/_thread.rs @@ -537,16 +537,14 @@ pub(crate) mod _thread { // would panic in push_thread_frame. { let func = func; - match func.invoke(args, vm) { - Ok(_obj) => {} - Err(e) if e.fast_isinstance(vm.ctx.exceptions.system_exit) => {} - Err(exc) => { - vm.run_unraisable( - exc, - Some("Exception ignored in thread started by".to_owned()), - func.into(), - ); - } + if let Err(exc) = func.invoke(args, vm) + && !exc.fast_isinstance(vm.ctx.exceptions.system_exit) + { + vm.run_unraisable( + exc, + Some("Exception ignored in thread started by".to_owned()), + func.into(), + ); } } for lock in SENTINELS.take() { @@ -1680,16 +1678,14 @@ pub(crate) mod _thread { { let func = func; // Run the function - match func.invoke((), vm) { - Ok(_) => {} - Err(e) if e.fast_isinstance(vm.ctx.exceptions.system_exit) => {} - Err(exc) => { - vm.run_unraisable( - exc, - Some("Exception ignored in thread started by".to_owned()), - func.into(), - ); - } + if let Err(exc) = func.invoke((), vm) + && !exc.fast_isinstance(vm.ctx.exceptions.system_exit) + { + vm.run_unraisable( + exc, + Some("Exception ignored in thread started by".to_owned()), + func.into(), + ); } } })) diff --git a/extra_tests/snippets/test_threading_teardown.py b/extra_tests/snippets/test_threading_teardown.py new file mode 100644 index 00000000000..ac3e088a777 --- /dev/null +++ b/extra_tests/snippets/test_threading_teardown.py @@ -0,0 +1,9 @@ +import threading +import time + +def runner(): + print('runner done') + +threading.Thread(target=runner).start() +time.sleep(1) +print('main done') From f862ed2c01d59d240cd37fdaa0b0cd431a3ad4f1 Mon Sep 17 00:00:00 2001 From: James David Clarke Date: Sun, 24 May 2026 09:50:37 +0100 Subject: [PATCH 3/6] Commit message {"subject": "Use double quotes in threading teardown test", "body": ""} --- extra_tests/snippets/test_threading_teardown.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extra_tests/snippets/test_threading_teardown.py b/extra_tests/snippets/test_threading_teardown.py index ac3e088a777..fa04ea2c20e 100644 --- a/extra_tests/snippets/test_threading_teardown.py +++ b/extra_tests/snippets/test_threading_teardown.py @@ -2,8 +2,8 @@ import time def runner(): - print('runner done') + print("runner done") threading.Thread(target=runner).start() time.sleep(1) -print('main done') +print("main done") From de594989f77014d62338c74ef69b3bbc5fec3843 Mon Sep 17 00:00:00 2001 From: James David Clarke Date: Sun, 24 May 2026 10:37:19 +0100 Subject: [PATCH 4/6] Move thread tests into stdlib_threading.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merge the contents of test_threading.py and test_threading_teardown.py into stdlib_threading.py to match the file's helper-function style; the two source snippets are deleted. The bodies become `thread_join_ordering` (start/join ordering) and `thread_exit_without_join` (regression test for #7813 — no-join thread teardown must not panic). Co-Authored-By: Claude Opus 4.7 (1M context) --- extra_tests/snippets/stdlib_threading.py | 43 +++++++++++++++++++ extra_tests/snippets/test_threading.py | 24 ----------- .../snippets/test_threading_teardown.py | 9 ---- 3 files changed, 43 insertions(+), 33 deletions(-) delete mode 100644 extra_tests/snippets/test_threading.py delete mode 100644 extra_tests/snippets/test_threading_teardown.py diff --git a/extra_tests/snippets/stdlib_threading.py b/extra_tests/snippets/stdlib_threading.py index f35d7e9d085..cb989e1fd33 100644 --- a/extra_tests/snippets/stdlib_threading.py +++ b/extra_tests/snippets/stdlib_threading.py @@ -1,6 +1,7 @@ import multiprocessing import os import threading +import time def import_in_thread(module_name): @@ -62,6 +63,48 @@ def start_fork_process_after_thread(): assert process.exitcode == 0, process.exitcode +def thread_join_ordering(): + output = [] + + def thread_function(name): + output.append((name, 0)) + time.sleep(2.0) + output.append((name, 1)) + + output.append((0, 0)) + x = threading.Thread(target=thread_function, args=(1,)) + output.append((0, 1)) + x.start() + output.append((0, 2)) + x.join() + output.append((0, 3)) + + assert len(output) == 6, output + # CPython has [(1, 0), (0, 2)] for the middle 2, but we have [(0, 2), (1, 0)] + # TODO: maybe fix this, if it turns out to be a problem? + # assert output == [(0, 0), (0, 1), (1, 0), (0, 2), (1, 1), (0, 3)] + + +def thread_exit_without_join(): + # Regression for https://github.com/RustPython/RustPython/issues/7813: + # a thread started without ``.join()`` must exit cleanly even when the + # captured target callable drops during teardown (which can fire + # weakref callbacks that re-enter the VM). + output = [] + + def runner(): + output.append("runner done") + + threading.Thread(target=runner).start() + time.sleep(1) + output.append("main done") + assert "runner done" in output, output + assert "main done" in output, output + + +thread_join_ordering() +thread_exit_without_join() + import_in_thread("functools") import_in_thread("tempfile") import_in_thread("multiprocessing.connection") diff --git a/extra_tests/snippets/test_threading.py b/extra_tests/snippets/test_threading.py deleted file mode 100644 index 4d7c29f5093..00000000000 --- a/extra_tests/snippets/test_threading.py +++ /dev/null @@ -1,24 +0,0 @@ -import threading -import time - -output = [] - - -def thread_function(name): - output.append((name, 0)) - time.sleep(2.0) - output.append((name, 1)) - - -output.append((0, 0)) -x = threading.Thread(target=thread_function, args=(1,)) -output.append((0, 1)) -x.start() -output.append((0, 2)) -x.join() -output.append((0, 3)) - -assert len(output) == 6, output -# CPython has [(1, 0), (0, 2)] for the middle 2, but we have [(0, 2), (1, 0)] -# TODO: maybe fix this, if it turns out to be a problem? -# assert output == [(0, 0), (0, 1), (1, 0), (0, 2), (1, 1), (0, 3)] diff --git a/extra_tests/snippets/test_threading_teardown.py b/extra_tests/snippets/test_threading_teardown.py deleted file mode 100644 index fa04ea2c20e..00000000000 --- a/extra_tests/snippets/test_threading_teardown.py +++ /dev/null @@ -1,9 +0,0 @@ -import threading -import time - -def runner(): - print("runner done") - -threading.Thread(target=runner).start() -time.sleep(1) -print("main done") From 29ae378f40f060a81e13edca8701427bb8373f94 Mon Sep 17 00:00:00 2001 From: James David Clarke Date: Sun, 24 May 2026 11:43:12 +0100 Subject: [PATCH 5/6] Add scheduled tasks lock file --- .claude/scheduled_tasks.lock | 1 + 1 file changed, 1 insertion(+) create mode 100644 .claude/scheduled_tasks.lock diff --git a/.claude/scheduled_tasks.lock b/.claude/scheduled_tasks.lock new file mode 100644 index 00000000000..12209032770 --- /dev/null +++ b/.claude/scheduled_tasks.lock @@ -0,0 +1 @@ +{"sessionId":"61ab036f-bd33-43a9-bd2d-2d4f272fcbb3","pid":23064,"procStart":"2860405","acquiredAt":1779614804287} \ No newline at end of file From 5e07d34d03f35e2b6a38a88dcb36b3e25a4e8a91 Mon Sep 17 00:00:00 2001 From: James David Clarke Date: Sun, 24 May 2026 18:58:10 +0100 Subject: [PATCH 6/6] Added .claude/scheduled_tasks.lock to .gitignore file --- .claude/scheduled_tasks.lock | 1 - .gitignore | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) delete mode 100644 .claude/scheduled_tasks.lock diff --git a/.claude/scheduled_tasks.lock b/.claude/scheduled_tasks.lock deleted file mode 100644 index 12209032770..00000000000 --- a/.claude/scheduled_tasks.lock +++ /dev/null @@ -1 +0,0 @@ -{"sessionId":"61ab036f-bd33-43a9-bd2d-2d4f272fcbb3","pid":23064,"procStart":"2860405","acquiredAt":1779614804287} \ No newline at end of file diff --git a/.gitignore b/.gitignore index 338a6437ca2..b5887be53b5 100644 --- a/.gitignore +++ b/.gitignore @@ -27,4 +27,4 @@ Lib/site-packages/* Lib/test/data/* !Lib/test/data/README cpython/ - +.claude/scheduled_tasks.lock \ No newline at end of file