Merge pull request #2523 from davidhewitt/init-once

pymodule: only allow initializing once per process

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<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
 

guide/src/migration.md

@@ -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

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)

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<ffi::PyModuleDef>,
     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::<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())
     }

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']");
     });