Skip to content
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

use GILProtected to synchronize access to various pyclass types #21670

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

tdyas
Copy link
Contributor

@tdyas tdyas commented Nov 19, 2024

As part of upgrading to PyO3 v0.23.x, access topyclass-annotated types will no longer be implicitly synchronized against the Python GIL. Those pyclass-annotated types must now be Sync and provide that synchronization explicitly.

Several places in Pants use RefCell which is not Send/Sync by itself. This PR uses pyo3::sync::GILProtected to provide explicit synchronization against the GIL for those use cases. (Eventually, we may wish to not use GILProtected to enable use of the Python 3.13 "no GIL" free threaded build, but that day is not today.)

Migration guide

@tdyas tdyas added category:internal CI, fixes for not-yet-released features, etc. release-notes:not-required PR doesn't require mention in release notes labels Nov 19, 2024
@tdyas tdyas requested review from cburroughs, benjyw and huonw November 19, 2024 12:23
@tdyas tdyas force-pushed the pyo3/gilprotected-1 branch from cefcb63 to 7095454 Compare November 19, 2024 13:22
@tdyas tdyas changed the title use GILProtected to synchronize access to various generator call structs use GILProtected to synchronize access to various pyclass types Nov 19, 2024
This was referenced Nov 19, 2024
@tdyas tdyas merged commit aeb1b74 into pantsbuild:main Nov 19, 2024
24 checks passed
@tdyas tdyas deleted the pyo3/gilprotected-1 branch November 19, 2024 21:06
tdyas added a commit that referenced this pull request Nov 21, 2024
[Upgrade to v0.23.x of the `pyo3`
crate](#21671):

- Functions in PyO3 with `_bound` suffixes existed in PyO3 only for
easing migration to the `Bound` API. They are deprecated now and new
methods without the `_bound` suffixes have been re-introduced in PyO3.
This PR renames call sites accordingly and updates code to also reflect
that some of the new APIs (e.g., `PyTuple::new`) are now fallible.

- The `IntoPy` and `ToPyObject` traits are deprecated in favor of the
new `IntoPyObject` trait (which is fallible). To ease migration, this PR
only disables deprecation warnings as errors in the affected files.
(Unfortunately, `IntoPyObject` was not introduced in the current v0.22.x
version and so we cannot migrate ahead of the upgrade (unlike what was
done in #21670 for the new
`pyclass` Sync requirement).) Migration will take place in follow-on
PRs.
 
- This PR does add an implementation of `IntoPyObject` for `&Value` to
support some existing call sites.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:internal CI, fixes for not-yet-released features, etc. release-notes:not-required PR doesn't require mention in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants