From a34bd4d72f0d5e79c706196705c122ba67d2d327 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Tue, 26 Jul 2022 19:28:03 +0100 Subject: [PATCH] pymodule: only allow initializing once per process --- CHANGELOG.md | 3 ++- guide/src/migration.md | 4 ++++ pytests/tests/test_misc.py | 18 ++++++++++++++++++ src/impl_/pymodule.rs | 16 +++++++++++++--- tests/test_module.rs | 32 ++++++++++++++++++++++---------- 5 files changed, 59 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b74ad94cce..f561d137355 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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>` instead of `&CStr`. - `FromPyObject::extract` now raises an error if source type is `PyString` and target type is `Vec`. [#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 diff --git a/guide/src/migration.md b/guide/src/migration.md index ba470719414..c65304718cc 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -77,6 +77,10 @@ fn get_type_object(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 diff --git a/pytests/tests/test_misc.py b/pytests/tests/test_misc.py index 406c55ee5be..8dfd06ba298 100644 --- a/pytests/tests/test_misc.py +++ b/pytests/tests/test_misc.py @@ -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) diff --git a/src/impl_/pymodule.rs b/src/impl_/pymodule.rs index 41f892946c1..28996b02e73 100644 --- a/src/impl_/pymodule.rs +++ b/src/impl_/pymodule.rs @@ -1,10 +1,13 @@ //! 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`. @@ -12,6 +15,7 @@ pub struct ModuleDef { // wrapped in UnsafeCell so that Rust compiler treats this as interior mutability ffi_def: UnsafeCell, initializer: ModuleInitializer, + initialized: AtomicBool, } /// Wrapper to enable initializer to be used in const fns. @@ -50,6 +54,7 @@ impl ModuleDef { ModuleDef { ffi_def, initializer, + initialized: AtomicBool::new(false), } } /// Builds a module using user given initializer. Used for [`#[pymodule]`][crate::pymodule]. @@ -57,6 +62,11 @@ impl ModuleDef { let module = unsafe { Py::::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()) } diff --git a/tests/test_module.rs b/tests/test_module.rs index 3acc2d7b659..61f6b1039ce 100644 --- a/tests/test_module.rs +++ b/tests/test_module.rs @@ -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"); @@ -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'"); }); @@ -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']"); });