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

pymodule: only allow initializing once per process #2523

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

davidhewitt
Copy link
Member

Following the suggestion in #2346 (comment) this changes #[pymodule] to raise an ImportError if initialised more than once. This seems to be a necessary defence against soundness issues related to sub-interpreters.

src/impl_/pymodule.rs Outdated Show resolved Hide resolved
@davidhewitt davidhewitt merged commit 8181160 into PyO3:main Aug 10, 2022
@davidhewitt davidhewitt deleted the init-once branch August 10, 2022 06:36
zer0def pushed a commit to zer0def/pyo3 that referenced this pull request Nov 8, 2024
Allow the subinterpreter safeguards to be disabled, so that applications
like Ceph's manager can continue to use pyo3 modules without soft
crashing.

Enabling this feature should be done with caution, as any storage of Py
objects in rust statics can lead to undefined behavior.

However, not all consumers of pyo3 use global state, and thus a subset
of them (such as python-bcrypt) are safe to use in subinterpreter
contexts.

References: bazaah/aur-ceph#20
References: PyO3#2523
References: pyca/cryptography#9016
References: PyO3#2346 (comment)
References: PyO3#2346 (comment)
References: PyO3#3451
Signed-off-by: Paul Stemmet <[email protected]>
@linas
Copy link

linas commented Dec 1, 2024

This change seems to have broken multiple downstream users; see for example pyca/cryptography#9016 and I'm trying to track the impacted projects here: pyca/cryptography#12080 It's kind of overwhelming because of the large amounts of chatter on so many different discussion groups and distro bug trackers that have been hit by this bug.

One possible fix is to apply this diff: zer0def@eba5243 which allows unsafe operation. Unclear if there's an open pull req for this, or whether it has been merged.

@davidhewitt
Copy link
Member Author

We should discuss this on #3451 which is the tracking issue for allowing sub interpreter use.

Even if we relaxed this check, newer Python interpreters would block PyO3 modules from being loaded because we don't yet use multi-phase init. #4162 is the latest attempt to do this. After that's landed we will need to update the PyO3 codebase to use module state, which needs some R&D.

I think a reachable mid to long-term goal is that we have the core PyO3 framework at least not invalid to use with subinterpreters, and an unsafe opt-in for users who are confident their module does not do prohibited activity under subinterpreters. There is a long discussion on #3451 why a safe solution is very hard.

@alex
Copy link
Contributor

alex commented Dec 3, 2024

FWIW, I think while it's probably not possible to have a fully safe sub-interpreter support (without huge perf overhead, at least), I do think if we can get to "pyo3 has no static state, and offers users a module state for their own stuff", then we can offer an unsafe "I promise I don't store any state statically" thing to allow sub-interpreters.

But the core work is:

  1. Multi-phase init
  2. Module state API

zer0def pushed a commit to zer0def/pyo3 that referenced this pull request Dec 12, 2024
Allow the subinterpreter safeguards to be disabled, so that applications
like Ceph's manager can continue to use pyo3 modules without soft
crashing.

Enabling this feature should be done with caution, as any storage of Py
objects in rust statics can lead to undefined behavior.

However, not all consumers of pyo3 use global state, and thus a subset
of them (such as python-bcrypt) are safe to use in subinterpreter
contexts.

References: bazaah/aur-ceph#20
References: PyO3#2523
References: pyca/cryptography#9016
References: PyO3#2346 (comment)
References: PyO3#2346 (comment)
References: PyO3#3451
Signed-off-by: Paul Stemmet <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants