Skip to content

Drop racy STOP_BIT assertion from eval-breaker test - #8789

Merged
youknowone merged 2 commits into
RustPython:mainfrom
youknowone:fix/sendall-deadline-stw-test
Sep 23, 2026
Merged

Drop racy STOP_BIT assertion from eval-breaker test#8789
youknowone merged 2 commits into
RustPython:mainfrom
youknowone:fix/sendall-deadline-stw-test

Conversation

@youknowone

@youknowone youknowone commented Sep 23, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Follow-up to #8786.

eval_breaker_tripped_when_stop_requested_and_stop_bit_clear failed on main Windows rust tests after merge: STOP_BIT or another process-wide breaker bit was set. The same job passed on the #8786 PR, so this is a parallel-test race.

STOP_BIT is process-wide. Other interpreter tests in the same binary flicker it, and this test both cleared that bit and asserted the eval-breaker word was empty. Clearing it from a unit test can also drop a stop that another test still needs.

The test now only sets the current thread's stop_requested and asserts eval_breaker_tripped() is true. It does not read or write the process-wide eval-breaker word.

Verified locally:

  • cargo test -p rustpython-vm --lib --features threading vm::interpreter::tests -- --test-threads=8 (29 tests, 5 repeats)

Assisted-by: Grok:4.6

Summary by CodeRabbit

  • Tests
    • Updated threading coverage to verify that a per-thread stop request trips the evaluation breaker.
    • Confirmed that handling a thread-specific stop request does not clear or inspect process-wide stop state.

@youknowone
Drop racy STOP_BIT assertion from eval-breaker test
f1fef34 
The isolated test asserted a clear process-wide eval breaker, and
cleared STOP_BIT itself. Parallel STW tests flicker that word, so
Windows rust tests failed after RustPython#8786.

Assisted-by: Grok:4.6
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 23, 2026

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: f18607a1-5167-4373-94a5-295eb348cf9e

📥 Commits

Reviewing files that changed from the base of the PR and between cb3d5db and f1fef34.

📒 Files selected for processing (1)
  • crates/vm/src/vm/interpreter.rs

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

📝 Walkthrough

Walkthrough

The threading test now focuses on the per-thread stop_requested path. It no longer clears or inspects process-wide stop state or pending-breaker state. It resets the request and verifies that the eval breaker trips.

Changes

Eval breaker stop request test

Layer / File(s) Summary
Per-thread stop request validation
crates/vm/src/vm/interpreter.rs		 The test was renamed to eval_breaker_tripped_when_stop_requested. It now validates only the per-thread stop request path and the resulting eval breaker trip.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 26731

The test now isolates per-thread stop requests, with no remaining merge-blocking risk identified.

🚥 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: removing a racy STOP_BIT assertion from the eval-breaker test.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch fix/sendall-deadline-stw-test
🧪 Generate unit tests (beta)
  • Create a new PR

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

Comment @coderabbitai help to get the list of available commands.

@youknowone
Merge branch 'main' into fix/sendall-deadline-stw-test
26731c2 
Keep the per-thread eval-breaker test. Drop the STOP_BIT retry
from RustPython#8788 and the unused stop_bit_set helper.

Assisted-by: Grok:4.6
@youknowone
youknowone merged commit e4984d5 into RustPython:main Sep 23, 2026
29 checks passed
@youknowone
youknowone deleted the fix/sendall-deadline-stw-test branch September 23, 2026 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone