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
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `PyCapsule::set_context` no longer takes a `py: Python<'_>` argument.
- `PyCapsule::name` now returns `PyResult<Option<&CStr>>` instead of `&CStr`.
- `FromPyObject::extract` now raises an error if source type is `PyString` and target type is `Vec<T>`. [#2500](https://github.com/PyO3/pyo3/pull/2500)
- `pyo3_build_config::add_extension_module_link_args()` now also emits linker arguments for `wasm32-unknown-emscripten`. [#2500](https://github.com/PyO3/pyo3/pull/2500)
- Only allow each `#[pymodule]` to be initialized once. [#2523](https://github.com/PyO3/pyo3/pull/2523)
- `pyo3_build_config::add_extension_module_link_args()` now also emits linker arguments for `wasm32-unknown-emscripten`. [#2538](https://github.com/PyO3/pyo3/pull/2538)

### Removed

Expand Down
4 changes: 4 additions & 0 deletions guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ fn get_type_object<T: PyTypeInfo>(py: Python<'_>) -> &PyType {

If this leads to errors, simply implement `IntoPy`. Because pyclasses already implement `IntoPy`, you probably don't need to worry about this.

### Each `#[pymodule]` can now only be initialized once per process

To make PyO3 modules sound in the presence of Python sub-interpreters, for now it has been necessary to explicitly disable the ability to initialize a `#[pymodule]` more than once in the same process. Attempting to do this will now raise an `ImportError`.

## from 0.15.* to 0.16

### Drop support for older technologies
Expand Down
18 changes: 18 additions & 0 deletions pytests/tests/test_misc.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,24 @@
import importlib
import platform

import pyo3_pytests.misc
import pytest


def test_issue_219():
# Should not deadlock
pyo3_pytests.misc.issue_219()


@pytest.mark.skipif(
platform.python_implementation() == "PyPy",
reason="PyPy does not reinitialize the module (appears to be some internal caching)",
)
def test_second_module_import_fails():
spec = importlib.util.find_spec("pyo3_pytests.pyo3_pytests")

with pytest.raises(
ImportError,
match="PyO3 modules may only be initialized once per interpreter process",
):
importlib.util.module_from_spec(spec)
16 changes: 13 additions & 3 deletions src/impl_/pymodule.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
//! Implementation details of `#[pymodule]` which need to be accessible from proc-macro generated code.
use std::cell::UnsafeCell;
use std::{
cell::UnsafeCell,
sync::atomic::{self, AtomicBool},
};

use crate::{
callback::panic_result_into_callback_output, ffi, types::PyModule, GILPool, IntoPyPointer, Py,
PyObject, PyResult, Python,
callback::panic_result_into_callback_output, exceptions::PyImportError, ffi, types::PyModule,
GILPool, IntoPyPointer, Py, PyObject, 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,
}

/// Wrapper to enable initializer to be used in const fns.
Expand Down Expand Up @@ -50,13 +54,19 @@ impl ModuleDef {
ModuleDef {
ffi_def,
initializer,
initialized: AtomicBool::new(false),
}
}
/// Builds a module using user given initializer. Used for [`#[pymodule]`][crate::pymodule].
pub fn make_module(&'static self, py: Python<'_>) -> PyResult<PyObject> {
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",
));
}
(self.initializer.0)(py, module.as_ref(py))?;
Ok(module.into())
}
Expand Down
32 changes: 22 additions & 10 deletions tests/test_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,17 +184,16 @@ fn custom_named_fn() -> usize {
42
}

#[pymodule]
fn foobar_module(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
m.add_function(wrap_pyfunction!(custom_named_fn, m)?)?;
m.dict().set_item("yay", "me")?;
Ok(())
}

#[test]
fn test_custom_names() {
#[pymodule]
fn custom_names(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
m.add_function(wrap_pyfunction!(custom_named_fn, m)?)?;
Ok(())
}

Python::with_gil(|py| {
let module = pyo3::wrap_pymodule!(foobar_module)(py);
let module = pyo3::wrap_pymodule!(custom_names)(py);

py_assert!(py, module, "not hasattr(module, 'custom_named_fn')");
py_assert!(py, module, "module.foobar() == 42");
Expand All @@ -203,8 +202,14 @@ fn test_custom_names() {

#[test]
fn test_module_dict() {
#[pymodule]
fn module_dict(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
m.dict().set_item("yay", "me")?;
Ok(())
}

Python::with_gil(|py| {
let module = pyo3::wrap_pymodule!(foobar_module)(py);
let module = pyo3::wrap_pymodule!(module_dict)(py);

py_assert!(py, module, "module.yay == 'me'");
});
Expand All @@ -213,7 +218,14 @@ fn test_module_dict() {
#[test]
fn test_module_dunder_all() {
Python::with_gil(|py| {
let module = pyo3::wrap_pymodule!(foobar_module)(py);
#[pymodule]
fn dunder_all(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
m.dict().set_item("yay", "me")?;
m.add_function(wrap_pyfunction!(custom_named_fn, m)?)?;
Ok(())
}

let module = pyo3::wrap_pymodule!(dunder_all)(py);

py_assert!(py, module, "module.__all__ == ['foobar']");
});
Expand Down