GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix stack overflow segfault on debug builds for Python threads by JamesClarke7283 · Pull Request #7986 · RustPython/RustPython

Conversation

JamesClarke7283 commented May 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Summary

Spawned Python threads were getting Rust's std::thread::Builder default 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 8 MB in apply_thread_stack_size whenever the user hasn't explicitly set a value via threading.stack_size(N). This matches CPython's effective default while keeping the Python API contract exact: threading.stack_size() still returns 0 ("platform default"), and explicit overrides still take precedence.
Single-file change in crates/vm/src/stdlib/_thread.rs.

Closes #7941

Test plan

cargo build succeeds.
Original reproducer passes on a debug build: cargo run -- -m test.test_ssl ThreadedTests.test_socketserver → Ran 1 test in 21.59s, OK (was segfaulting on main).
Python API contract preserved: threading.stack_size() returns 0 by default; setting to 1 MiB then reading back returns 1048576; previous value reported correctly.
Basic threaded workload (5 threads doing work + join) still completes normally.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Made thread stack sizing more predictable in debug builds by using a consistent fallback when no size is configured.
- Applied configured stack sizes to newly spawned threads.
- Release builds continue using the platform default when no stack size is set.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Fix stack overflow segfault on debug builds for Python threads

ea04e96

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) <[email protected]>

coderabbitai Bot commented May 27, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Last reviewed commit: a5e5bc6
Latest commit, not reviewed: 26faa24

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: f0746f90-6ed3-42ff-8dc1-1eea0615173c

📥 Commits

Reviewing files that changed from the base of the PR and between 41b4f73 and a5e5bc6.

📒 Files selected for processing (1)

crates/vm/src/stdlib/_thread.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.

Walkthrough

When vm.state.stacksize is non-zero, apply_thread_stack_size applies it. When it is zero, debug builds use an 8 MiB stack and release builds use Rust’s default. A platform-specific regression test verifies the debug fallback.

Changes: Thread Stack Sizing

Layer / File(s)	Summary
Stack-size selection logic `crates/vm/src/stdlib/_thread.rs`	Adds the debug-only `DEFAULT_THREAD_STACK_SIZE` constant. `apply_thread_stack_size` handles configured, debug, and release stack-size cases.
Debug stack-size regression test `crates/vm/src/stdlib/_thread.rs`	Adds Linux and macOS helpers that measure the spawned thread stack size. The test checks that debug builds provide at least 8 MiB.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: youknowone

Merge Risk: Minimal · previous review a5e5b

The debug default stack fallback is implemented while explicit Python stack-size settings retain precedence. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing stack-overflow segmentation faults for Python threads in debug builds.
Linked Issues check	✅ Passed	Issue `#7941` requires Python threaded tests to avoid debug-build stack overflows and segmentation faults. The PR applies an 8 MiB stack when no explicit stack size is configured, while preserving expli…
Out of Scope Changes check	✅ Passed	The changes are limited to `crates/vm/src/stdlib/_thread.rs`. They implement the debug-build thread-stack fix for `#7941` and add its regression test. The changes do not add unrelated behavior.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files.

Finishing Touches

Resolve merge conflicts

Resolve merge conflict in branch fix-7941-thread-stack-overflow

🧪 Generate unit tests (beta)

Create PR with unit tests

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JamesClarke7283 and others added 3 commits

May 27, 2026 09:47


          Add doc comment to apply_thread_stack_size

5a313d4

Satisfy docstring coverage check on the PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>


          Retrigger CI for flaky multiprocessing tests

f8bfb43

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>


          Scope thread stack size override to debug builds only

42f519f

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) <[email protected]>

youknowone commented May 28, 2026

Copy link

Copy Markdown

Member

please also add failing test before patch

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JamesClarke7283 marked this pull request as draft

June 3, 2026 13:59

JamesClarke7283 marked this pull request as ready for review

June 3, 2026 17:02

This comment was marked as outdated.

JamesClarke7283 commented Jun 3, 2026

Copy link

Copy Markdown

Contributor Author

please also add failing test before patch

Which test? the original one from the original issue that now passes or something different.
Please clarify.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Merge branch 'main' into fix-7941-thread-stack-overflow

b9483d7

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) <[email protected]>

youknowone commented Aug 9, 2026

Copy link

Copy Markdown

Member

please add a test, which failed before the patch but success after the patch. If adding patch is not possible, please share discussion why it is impossible

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Add regression test for debug thread stack size

a5e5bc6

Verifies that a Python thread started without an explicit
threading.stack_size() gets DEFAULT_THREAD_STACK_SIZE (8 MiB) in
debug builds instead of Rust's 2 MiB std default, which overflowed
and crashed in the SSL test server (RustPython#7941). The test reads back the
spawned thread's actual stack size via pthread, fails before the
fix and passes after it.

JamesClarke7283 commented Aug 9, 2026

Copy link

Copy Markdown

Contributor Author

please add a test, which failed before the patch but success after the patch. If adding patch is not possible, please share discussion why it is impossible

Done

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

youknowone force-pushed the main branch from d1c4925 to 3191eca Compare

September 16, 2026 17:13

youknowone force-pushed the fix-7941-thread-stack-overflow branch from 54f5615 to a5e5bc6 Compare

September 16, 2026 17:17


          Merge main into fix-7941-thread-stack-overflow

26faa24

Keep main's integrated debug stack fallback and regression coverage, including the minimum stack size for explicit requests. This resolves the conflicting older implementation without undoing upstream improvements.

AI assistance: Codex resolved the merge and reviewed the stack-size implementation.

Assisted-by: Codex:GPT-6

codspeed Bot commented Oct 31, 2026

Copy link

Copy Markdown

Contributor

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks¹

_{Comparing JamesClarke7283:fix-7941-thread-stack-overflow (26faa24) with main (713e14c)}

Footnotes

4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Some python tests are crashing with segmentation fault when running on debug build

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

JamesClarke7283 commented May 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Uh oh!

youknowone commented May 28, 2026

Uh oh!

This comment was marked as outdated.

JamesClarke7283 commented Jun 3, 2026

Uh oh!

youknowone commented Aug 9, 2026

Uh oh!

JamesClarke7283 commented Aug 9, 2026

Uh oh!

codspeed Bot commented Oct 31, 2026

Merging this PR will not alter performance

Footnotes

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JamesClarke7283 commented May 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented May 27, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.