Skip to content

[python] Ensure GIL is held during CPython API call#22402

Merged
vepadulano merged 1 commit into
root-project:masterfrom
vepadulano:py-hold-gil-during-cpython-call
May 27, 2026
Merged

[python] Ensure GIL is held during CPython API call#22402
vepadulano merged 1 commit into
root-project:masterfrom
vepadulano:py-hold-gil-during-cpython-call

Conversation

@vepadulano

Copy link
Copy Markdown
Member

ROOT's CPython extension defines a custom error handler that tries to use Python standard warnings when possible. In such cases, the CPython API is called and thus the GIL must be held. If the calling Python function had previously released the GIL it will result in trouble. This is the case for the TH1.Fit method which has been changed to always releasing the GIL, under the assumption that everything happening during the call happens in C++, but that's not true. This commit proposes to restore the GIL for the call to the CPython API to raise the warning.

@vepadulano vepadulano self-assigned this May 25, 2026
@vepadulano vepadulano requested a review from guitargeek as a code owner May 25, 2026 05:34
@vepadulano vepadulano linked an issue May 25, 2026 that may be closed by this pull request
1 task
@github-actions

github-actions Bot commented May 25, 2026

Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 12h 45m 1s ⏱️
 3 859 tests  3 859 ✅ 0 💤 0 ❌
77 063 runs  77 063 ✅ 0 💤 0 ❌

Results for commit fc87fa7.

♻️ This comment has been updated with latest results.

@dpiparo dpiparo added this to the 6.40.02 milestone May 25, 2026

@guitargeek guitargeek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for the fix!

ROOT's CPython extension defines a custom error handler that tries to use Python standard warnings when possible. In such cases, the CPython API is called and thus the GIL must be held. If the calling Python function had previously released the GIL it will result in trouble. This is the case for the TH1.Fit method which has been changed to always releasing the GIL, under the assumption that everything happening during the call happens in C++, but that's not true. This commit proposes to restore the GIL for the call to the CPython API to raise the warning.
@vepadulano vepadulano force-pushed the py-hold-gil-during-cpython-call branch from 39eeb7d to fc87fa7 Compare May 27, 2026 03:02
@vepadulano vepadulano merged commit 56ce5ff into root-project:master May 27, 2026
33 checks passed
@vepadulano

Copy link
Copy Markdown
Member Author

/backport to 6.40

@root-project-bot

Copy link
Copy Markdown

Preparing to backport PR #22402 to branch 6.40 requested by vepadulano

@root-project-bot

Copy link
Copy Markdown

This PR has been backported to branch 6.40: #22408

wdconinc added a commit to eic/eic-spack that referenced this pull request Jun 3, 2026
### Briefly, what does this PR introduce? Please link to any relevant
presentations or discussions.
This PR backports root-project/root#22402 into
6.38.02 through 6.40.00, in order to avoid failures in
https://eicweb.phy.anl.gov/EIC/benchmarks/detector_benchmarks/-/jobs/7889057.

### What is the urgency of this PR?
- [ ] High (please describe reason below)
- [x] Medium
- [ ] Low

### What kind of change does this PR introduce?
- [x] Bug fix (issue:
https://eicweb.phy.anl.gov/EIC/benchmarks/detector_benchmarks/-/jobs/7889057)
- [ ] New feature (issue #__)
- [ ] Optimization (issue #__)
- [ ] Updated documentation
- [ ] other: __

### Please check if any of the following apply
- [ ] This PR introduces breaking changes. Please describe changes users
need to make below.
- [ ] This PR changes default behavior. Please describe changes below.
- [ ] AI was used in preparing this PR. Please describe usage below.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash in PyROOT in 6.40 (GIL issue?)

4 participants