Skip to content

[cppyy] Correct management of GIL in concurrency tests #19237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 1, 2025

Conversation

guitargeek
Copy link
Contributor

The GIL should always be acquired via the thread state API when doing Python calls from multithreaded C++.

Fixes the test failure with Python debug builds.

Copy link

github-actions bot commented Jul 1, 2025

Test Results

    20 files      20 suites   3d 6h 6m 6s ⏱️
 3 066 tests  3 066 ✅ 0 💤 0 ❌
59 674 runs  59 674 ✅ 0 💤 0 ❌

Results for commit eff7147.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@Vipul-Cariappa Vipul-Cariappa left a comment

Choose a reason for hiding this comment

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

I guess the changes require PyGILState_Ensure & PyGILState_Release.
Reference: https://docs.python.org/3/c-api/init.html#non-python-created-threads

The GIL should always be acquired via the thread state API when doing
Python calls from multithreaded C++.

Fixes the test failure with Python debug builds.
Copy link
Contributor

@Vipul-Cariappa Vipul-Cariappa left a comment

Choose a reason for hiding this comment

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

LGTM!

@guitargeek guitargeek merged commit caf30ad into root-project:master Jul 1, 2025
24 of 25 checks passed
@guitargeek guitargeek deleted the issue-19224 branch July 1, 2025 09:50
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.

2 participants