Fix stack overflow segfault on debug builds for Python threads#7986
Fix stack overflow segfault on debug builds for Python threads#7986JamesClarke7283 wants to merge 5 commits into
Conversation
Rust's std::thread::Builder defaults to a 2 MB stack when no size is
set, which is too small for the call chains the Python stdlib runs on
helper threads in debug builds (e.g. test.test_ssl's threaded server).
CPython on glibc Linux relies on pthread's ~8 MB default instead.
Apply 8 MB as the default in apply_thread_stack_size when the user has
not explicitly called threading.stack_size(N). This matches CPython's
effective default across builds while preserving the existing API:
threading.stack_size() still returns 0 by default ("platform default"),
and explicit user values still take precedence.
Fixes RustPython#7941
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughWhen starting helper threads, apply_thread_stack_size uses ChangesThread Stack Sizing
Sequence Diagram(s)sequenceDiagram
participant Caller
participant apply_thread_stack_size
participant thread_Builder
Caller->>apply_thread_stack_size: pass vm.state.stacksize
alt vm.state.stacksize != 0
apply_thread_stack_size->>thread_Builder: stack_size(configured)
else vm.state.stacksize == 0 (debug)
apply_thread_stack_size->>thread_Builder: stack_size(DEFAULT_THREAD_STACK_SIZE)
else vm.state.stacksize == 0 (release)
apply_thread_stack_size-->>thread_Builder: no stack_size call (std default)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Satisfy docstring coverage check on the PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit applied an 8 MB default unconditionally, which slowed down multiprocessing tests on release: many forked children each spawning many threads with oversized virtual stack mappings caused the flaky MP CI step to time out (60 min, vs ~9 min on main). Issue RustPython#7941 only manifested in debug builds — release builds were already fine on Rust's 2 MB std default. Restrict the 8 MB override to `#[cfg(debug_assertions)]` so release behavior is unchanged from before the fix, while the original SSL segfault on debug stays fixed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
please also add failing test before patch |
This comment was marked as outdated.
This comment was marked as outdated.
Which test? the original one from the original issue that now passes or something different. |
Brings the branch up to date with main so CI job names match the required status checks (the windows snippets/cpython job was renamed by RustPython#8004 when its matrix skips were cleared). Also picks up RustPython#8018's _thread.rs stack-margin updates, which are orthogonal to the apply_thread_stack_size fix in this PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
std::thread::Builderdefault stack of 2 MB, which is too small for the call chains the Python stdlib runs on helper threads in debug builds (where Rust stack frames are substantially larger). CPython on glibc Linux relies on pthread's ~8 MB default instead — a 4× difference that explains why this hits us but not CPython.apply_thread_stack_sizewhenever the user hasn't explicitly set a value viathreading.stack_size(N). This matches CPython's effective default while keeping the Python API contract exact:threading.stack_size()still returns0("platform default"), and explicit overrides still take precedence.crates/vm/src/stdlib/_thread.rs.Closes #7941
Test plan
cargo buildsucceeds.cargo run -- -m test.test_ssl ThreadedTests.test_socketserver→Ran 1 test in 21.59s, OK(was segfaulting onmain).threading.stack_size()returns0by default; setting to 1 MiB then reading back returns1048576; previous value reported correctly.🤖 Generated with Claude Code
Summary by CodeRabbit