Skip to content

Keep SemLock OS calls behind SemHandle methods - #8740

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:fix/flaky-mp
Sep 19, 2026
Merged

youknowone merged 1 commit into
RustPython:mainfrom
youknowone:fix/flaky-mp

Conversation

@youknowone

@youknowone youknowone commented Sep 19, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Stop exporting libc semaphore pointers from host_env.
  • stdlib waits, posts, and reads values through SemHandle instead of *mut sem_t / HANDLE.
  • Blocking waits still drop the owner-count mutex; only the OS handle API is wrapped.

Follow-up to #8739.

Test plan

  • cargo check -p rustpython-host_env
  • cargo check -p rustpython-stdlib
  • CI

Summary by CodeRabbit

  • Improvements
    • Improved multiprocessing semaphore handling across Unix and Windows platforms.
    • Added more consistent support for waiting, polling, releasing, and retrieving semaphore values.
    • Preserved existing timeout, signal, ownership, overflow, and operating-system error behavior.
    • Improved deadline handling for timed semaphore operations, including platform-specific polling support.

Stop exporting libc semaphore pointers from host_env. Stdlib
now waits, posts, and reads values through SemHandle instead
of *mut sem_t / HANDLE.

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

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 19, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

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: 57545654-b4c7-47e6-a7a5-d193234ffe10

📥 Commits

Reviewing files that changed from the base of the PR and between 0bdb61b and 7842566.

📒 Files selected for processing (2)
  • crates/host_env/src/multiprocessing.rs
  • crates/stdlib/src/multiprocessing.rs

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


📝 Walkthrough

Walkthrough

The change moves semaphore operations behind platform-specific SemHandle methods. It adds the Unix Deadline wrapper, makes low-level helpers private, and updates Windows, Unix, and macOS SemLock paths to use the new abstraction.

Changes

Semaphore handle encapsulation

Layer / File(s) Summary
Host SemHandle API
crates/host_env/src/multiprocessing.rs
SemHandle gains platform-specific wait, post, release, value, polling, and handle conversion methods. Unix deadlines use the public Deadline wrapper. Low-level helpers and libc types are no longer publicly exported.
Windows semaphore integration
crates/stdlib/src/multiprocessing.rs
Windows SemLock uses SemHandle for handle conversion, waiting, releasing, and value checks.
Unix and macOS semaphore integration
crates/stdlib/src/multiprocessing.rs
Unix and macOS SemLock uses SemHandle for waiting, posting, value retrieval, handle conversion, and timed polling.

Priority: ⬇️ Low

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

Change: Refactor

Suggested reviewers: joshuamegnauth54

🚥 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: moving semaphore operating-system calls behind SemHandle methods.
Docstring Coverage ✅ Passed Docstring coverage is 96.88% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 2 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
🧪 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
youknowone merged commit 6d39d13 into RustPython:main Sep 19, 2026
30 checks passed
@youknowone
youknowone deleted the fix/flaky-mp branch September 19, 2026 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant