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

relax multiple-import check to only prevent subinterpreters #3446

Merged
merged 3 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion guide/src/building_and_distribution.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ If you encounter these or other complications when linking the interpreter stati

### Import your module when embedding the Python interpreter

When you run your Rust binary with an embedded interpreter, any `#[pymodule]` created modules won't be accessible to import unless added to a table called `PyImport_Inittab` before the embedded interpreter is initialized. This will cause Python statements in your embedded interpreter such as `import your_new_module` to fail. You can call the macro [`append_to_inittab`]({{#PYO3_DOCS_URL}}/pyo3/macro.append_to_inittab.html) with your module before initializing the Python interpreter to add the module function into that table. (The Python interpreter will be initialized by calling `prepare_freethreaded_python`, `with_embedded_interpreter`, or `Python::with_gil` with the [`auto-initialize`](features.md#auto-initialize) feature enabled.)
When you run your Rust binary with an embedded interpreter, any `#[pymodule]` created modules won't be accessible to import unless added to a table called `PyImport_Inittab` before the embedded interpreter is initialized. This will cause Python statements in your embedded interpreter such as `import your_new_module` to fail. You can call the macro [`append_to_inittab`]({{#PYO3_DOCS_URL}}/pyo3/macro.append_to_inittab.html) with your module before initializing the Python interpreter to add the module function into that table. (The Python interpreter will be initialized by calling `prepare_freethreaded_python`, `with_embedded_python_interpreter`, or `Python::with_gil` with the [`auto-initialize`](features.md#auto-initialize) feature enabled.)

## Cross Compiling

Expand Down
1 change: 1 addition & 0 deletions newsfragments/3446.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`#[pymodule]` will now return the same module object on repeated import by the same Python interpreter, on Python 3.9 and up.
33 changes: 27 additions & 6 deletions pytests/tests/test_misc.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import importlib
import platform
import sys

import pyo3_pytests.misc
import pytest
Expand All @@ -10,15 +11,35 @@ def test_issue_219():
pyo3_pytests.misc.issue_219()


@pytest.mark.xfail(
platform.python_implementation() == "CPython" and sys.version_info < (3, 9),
reason="Cannot identify subinterpreters on Python older than 3.9",
)
def test_multiple_imports_same_interpreter_ok():
spec = importlib.util.find_spec("pyo3_pytests.pyo3_pytests")

module = importlib.util.module_from_spec(spec)
assert dir(module) == dir(pyo3_pytests.pyo3_pytests)


@pytest.mark.xfail(
platform.python_implementation() == "CPython" and sys.version_info < (3, 9),
reason="Cannot identify subinterpreters on Python older than 3.9",
)
@pytest.mark.skipif(
platform.python_implementation() == "PyPy",
reason="PyPy does not reinitialize the module (appears to be some internal caching)",
reason="PyPy does not support subinterpreters",
)
def test_second_module_import_fails():
spec = importlib.util.find_spec("pyo3_pytests.pyo3_pytests")
def test_import_in_subinterpreter_forbidden():
import _xxsubinterpreters

sub_interpreter = _xxsubinterpreters.create()
with pytest.raises(
ImportError,
match="PyO3 modules may only be initialized once per interpreter process",
_xxsubinterpreters.RunFailedError,
match="PyO3 modules do not yet support subinterpreters, see https://github.com/PyO3/pyo3/issues/576",
):
importlib.util.module_from_spec(spec)
_xxsubinterpreters.run_string(
sub_interpreter, "import pyo3_pytests.pyo3_pytests"
)

_xxsubinterpreters.destroy(sub_interpreter)
78 changes: 62 additions & 16 deletions src/impl_/pymodule.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
//! Implementation details of `#[pymodule]` which need to be accessible from proc-macro generated code.

use std::{
cell::UnsafeCell,
sync::atomic::{self, AtomicBool},
};
use std::cell::UnsafeCell;

use crate::{exceptions::PyImportError, ffi, types::PyModule, Py, PyResult, Python};
#[cfg(all(not(PyPy), Py_3_9))]
use std::sync::atomic::{AtomicI64, Ordering};

#[cfg(not(PyPy))]
use crate::exceptions::PyImportError;
use crate::{ffi, sync::GILOnceCell, types::PyModule, Py, PyResult, Python};

/// `Sync` wrapper of `ffi::PyModuleDef`.
pub struct ModuleDef {
// wrapped in UnsafeCell so that Rust compiler treats this as interior mutability
ffi_def: UnsafeCell<ffi::PyModuleDef>,
initializer: ModuleInitializer,
initialized: AtomicBool,
/// Interpreter ID where module was initialized (not applicable on PyPy).
#[cfg(all(not(PyPy), Py_3_9))]
interpreter: AtomicI64,
/// Initialized module object, cached to avoid reinitialization.
module: GILOnceCell<Py<PyModule>>,
}

/// Wrapper to enable initializer to be used in const fns.
Expand Down Expand Up @@ -51,7 +57,10 @@ impl ModuleDef {
ModuleDef {
ffi_def,
initializer,
initialized: AtomicBool::new(false),
// -1 is never expected to be a valid interpreter ID
#[cfg(all(not(PyPy), Py_3_9))]
interpreter: AtomicI64::new(-1),
module: GILOnceCell::new(),
}
}
/// Builds a module using user given initializer. Used for [`#[pymodule]`][crate::pymodule].
Expand All @@ -71,16 +80,53 @@ impl ModuleDef {
))?;
}
}
let module = unsafe {
Py::<PyModule>::from_owned_ptr_or_err(py, ffi::PyModule_Create(self.ffi_def.get()))?
};
if self.initialized.swap(true, atomic::Ordering::SeqCst) {
return Err(PyImportError::new_err(
"PyO3 modules may only be initialized once per interpreter process",
));
// Check the interpreter ID has not changed, since we currently have no way to guarantee
// that static data is not reused across interpreters.
//
// PyPy does not have subinterpreters, so no need to check interpreter ID.
#[cfg(not(PyPy))]
{
#[cfg(Py_3_9)]
{
let current_interpreter =
unsafe { ffi::PyInterpreterState_GetID(ffi::PyInterpreterState_Get()) };
crate::err::error_on_minusone(py, current_interpreter)?;
if let Err(initialized_interpreter) = self.interpreter.compare_exchange(
-1,
current_interpreter,
Ordering::SeqCst,
Ordering::SeqCst,
) {
if initialized_interpreter != current_interpreter {
return Err(PyImportError::new_err(
"PyO3 modules do not yet support subinterpreters, see https://github.com/PyO3/pyo3/issues/576",
));
}
}
}
#[cfg(not(Py_3_9))]
{
// CPython before 3.9 does not have APIs to check the interpreter ID, so best that can be
// done to guard against subinterpreters is fail if the module is initialized twice
if self.module.get(py).is_some() {
return Err(PyImportError::new_err(
"PyO3 modules compiled for CPython 3.8 or older may only be initialized once per interpreter process"
));
}
}
}
(self.initializer.0)(py, module.as_ref(py))?;
Ok(module)
self.module
.get_or_try_init(py, || {
let module = unsafe {
Py::<PyModule>::from_owned_ptr_or_err(
py,
ffi::PyModule_Create(self.ffi_def.get()),
)?
};
(self.initializer.0)(py, module.as_ref(py))?;
Ok(module)
})
.map(|py_module| py_module.clone_ref(py))
}
}

Expand Down
Loading