{{ message }}
ssl: pass test_ssl with the rustls backend - #8502
Merged
Merged
Conversation
Assisted-by: OpenAI Codex:GPT-5
Contributor
youknowone
marked this pull request as ready for review
August 12, 2026 07:17
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/vm/src/stdlib/_thread.rs (1)
2034-2047: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the explicit stack-size path.
The test only covers
vm.state.stacksize == 0. Add a debug Linux/macOS test that sets a value larger thanDEFAULT_THREAD_STACK_SIZEand verifies that the observed native stack size is at least that configured value. This verifies that an explicitthreading.stack_size()value continues to override the debug default.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/vm/src/stdlib/_thread.rs` around lines 2034 - 2047, Add a debug Linux/macOS test alongside default_python_thread_stack_size_debug that sets vm.state.stacksize to a value larger than DEFAULT_THREAD_STACK_SIZE before calling apply_thread_stack_size, then spawn and join current_thread_stack_size and assert the observed size is at least the configured value. Keep the test scoped to the supported debug platforms and verify the explicit stack-size setting overrides the default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/vm/src/stdlib/_thread.rs`:
- Around line 2034-2047: Add a debug Linux/macOS test alongside
default_python_thread_stack_size_debug that sets vm.state.stacksize to a value
larger than DEFAULT_THREAD_STACK_SIZE before calling apply_thread_stack_size,
then spawn and join current_thread_stack_size and assert the observed size is at
least the configured value. Keep the test scoped to the supported debug
platforms and verify the explicit stack-size setting overrides the default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: b11c80dd-21cb-4f69-a121-1691a32525ec
📒 Files selected for processing (3)
crates/stdlib/src/ssl.rscrates/stdlib/src/ssl/cert.rscrates/vm/src/stdlib/_thread.rs
Assisted-by: OpenAI Codex:GPT-5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.

Summary
test_sslbehaviorRoot cause
Several rustls compatibility details diverged from the behavior expected by CPython's SSL tests. In addition, Rust's 2 MiB default worker-thread stack is too small for RustPython's unoptimized debug call chains: the threaded SSL server reached the native stack guard at a Python recursion depth of only 21. The stack override is debug-only, preserves explicit
threading.stack_size()values, and leaves release behavior unchanged.Impact
cargo run -- -m test test_sslnow completes successfully with all 196 enabled tests passing. The changes keep the non-OpenSSL rustls/AWS-LC implementation and improve handshake correctness and certificate compatibility.Validation
cargo run -- -m test test_sslcargo run --release -- -m test test_ssl test_threadcargo test -p rustpython-vm --features threading default_python_thread_stack_size_debug -- --nocapturecargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi(cd crates/capi && cargo test)cargo fmt --all -- --checkcargo clippy --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi --all-targetsThe clippy run completed with only five pre-existing
must_use_candidatewarnings inrustpython-compiler-source.Summary by CodeRabbit