signal: describe what the OS handler actually writes, and cover duck-typed re group selectors

[youknowone]

Two independent follow-ups from the #1209 review.

signal: the ticker comments named the wrong writer

Three comments — register_ticker, signal_pushback, and the ticker_addr handoff in install_signal_handling — documented the OS signal handler as forcing the ActionFlag ticker negative. It does not. The handler sets the pending bit and arms the eval-breaker's async bit, both lock-free atomics; sync_async_ticker drives the ticker negative later, under the GIL. That split is exactly what keeps the handler async-signal-safe, so the comments now say why the handler must not write that cell.

Comments only; no behaviour change. A reviewer named one of the three sites — the other two carried the same claim.

re: __index__ arm of the group-selector conversion was uncovered

group, __getitem__, start, end, and span take a selector as a number when its type supplies __index__ and as a name otherwise. stdlib_re.py only ever passed literal integers and literal names, so the conversion's __index__ arm — which runs arbitrary Python — was never exercised.

Not included

The review's Critical finding on sre_match_group (an unrooted *const W_SRE_Match held across do_span, which can run arbitrary Python) is refuted. W_SRE_Match is built only by w_sre_match_new → W_SRE_Match::allocate_stable → alloc_in_oldgen; MiniMark's mark-sweep never moves old-gen objects, and drag_out_root relocates only is_nursery_object_start refs. allocate_stable exists precisely so callers can hold the raw pointer on the Rust stack without rooting it. I did write that "fix" before checking the allocator, then reverted it.

[coderabbitai]

Warning

Review limit reached

@youknowone, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 34 seconds

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 47206ecf-85fc-494a-9d7b-1b82fbf3050c

📥 Commits

Reviewing files that changed from the base of the PR and between 45d3eba and efcb5ef.

📒 Files selected for processing (3)

• pyre/extra_tests/snippets/stdlib_re.py
• pyre/pyre-interpreter/src/module/signal/interp_signal.rs
• pyre/pyre-interpreter/src/module/signal/signalstate.rs

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit efcb5ef).
Updated: 2026-08-14T15:09:11.868Z

Files in the reviewed diff

pyre/extra_tests/snippets/stdlib_re.py
pyre/pyre-interpreter/src/module/signal/interp_signal.rs
pyre/pyre-interpreter/src/module/signal/signalstate.rs

1. Regressions to PyPy parity introduced by this patch

None.

2. Other mismatches introduced by this patch

None.

3. Pre-existing mismatches (already present before this patch)

None.

4. Structural adaptations

• pyre/pyre-interpreter/src/module/signal/signalstate.rs:83 ↔ rpython/translator/c/src/signals.c:112: pyre arms an atomic eval-breaker and transfers it to the ticker at a safe checkpoint, whereas PyPy’s C handler writes the signal ticker directly. This is a Rust async-signal-safety / implementation-language adaptation, not a parity regression. The executable behavior predates this patch; the patch only corrects its documentation.