Skip to content

Add recursion guard to the c-api - #8796

Open
bschoenmaeckers wants to merge 3 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-recursion
Open

Add recursion guard to the c-api#8796
bschoenmaeckers wants to merge 3 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-recursion

Conversation

@bschoenmaeckers

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

Copy link
Copy Markdown
Contributor
  • Closes #xxxx

One of checkbox below must be checked.

  • I did not use AI to write the code of this patch.
  • This PR follows our AI policy

Summary

Summary by CodeRabbit

  • New Features
    • Native extensions can participate in recursion tracking, with calls checked against the runtime’s recursion limits and excessive recursion reported as a recursion error.
  • Bug Fixes
    • Recursion depth is restored when calls finish or unwind. Stack-depth checks account for platform differences, helping prevent excessive native recursion and stack exhaustion.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 23, 2026

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@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.

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.

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: Repository: RustPython/RustPython/.coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: 1f0f0768-1716-4240-a41d-cdc3e5f11732

📥 Commits

Reviewing files that changed from the base of the PR and between b1053b8 and 2262a5d.

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

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

📝 Walkthrough

Walkthrough

The VM now exposes methods to enter and leave recursive calls. The C API exports matching functions. The with_recursion path uses the VM methods and a scope guard to leave on scope exit. A C API test checks that recursion reports a PyRecursionError.

Changes

Recursive call tracking

Layer / File(s) Summary
VM recursive-call tracking
crates/vm/src/vm/mod.rs		 The VM adds entry and leave methods. with_recursion calls them and uses a scope guard to leave when the scope exits.
C API exports
crates/capi/src/ceval.rs		 The C API adds Py_EnterRecursiveCall and Py_LeaveRecursiveCall, which delegate to the VM methods. The enter function converts its call-site string. The test checks that recursion reports a PyRecursionError.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CCaller
  participant CAPI
  participant VirtualMachine
  CCaller->>CAPI: call Py_EnterRecursiveCall(where)
  CAPI->>VirtualMachine: enter_recursive_call(where)
  VirtualMachine-->>CAPI: return result
  CCaller->>CAPI: call Py_LeaveRecursiveCall()
  CAPI->>VirtualMachine: leave_recursive_call()
Loading

Suggested reviewers: youknowone

Merge Risk: ⚪ Minimal · up to 2262a

No actionable merge-blocking risk is established; the change is mergeable after normal checks.

🚥 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 accurately identifies the main change: adding a recursion guard to the C API. It is concise and specific enough for repository history.
Docstring Coverage ✅ Passed Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 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 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 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.

@bschoenmaeckers
bschoenmaeckers force-pushed the c-api-recursion branch 2 times, most recently from 83dac60 to c2c18e7 Compare September 23, 2026 11:53
@bschoenmaeckers

bschoenmaeckers commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

Not sure why this breaks on only linux.

@codspeed

codspeed Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1

Comparing bschoenmaeckers:c-api-recursion (c2c18e7) with main (43a595e)

Open in CodSpeed

Footnotes

  1. 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. ↩

@bschoenmaeckers
bschoenmaeckers force-pushed the c-api-recursion branch 2 times, most recently from e5f7347 to 2262a5d Compare September 23, 2026 15:38
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

@bschoenmaeckers