Skip to content

Commit

Permalink
pymodule: only allow initializing once per process
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Aug 9, 2022
1 parent 10a87ac commit 78ba70d
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 14 deletions.
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

0 comments on commit 78ba70d

Please sign in to comment.