From 22ebf7d9c2a51bcceae7877106362ecaa7987224 Mon Sep 17 00:00:00 2001 From: Gregory Szorc Date: Wed, 30 Dec 2020 12:12:56 -0700 Subject: [PATCH] pyembed: adjust lifetime return value from acquire_gil() See the inline comments for more. Closes #345. --- docs/history.rst | 3 +++ pyembed/src/interpreter.rs | 7 ++++++- pyembed/src/test/main_python_interpreter.rs | 13 +++++-------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/docs/history.rst b/docs/history.rst index c59121549..431cf73a4 100644 --- a/docs/history.rst +++ b/docs/history.rst @@ -69,6 +69,9 @@ Bug Fixes to be equivalent to the parent package to partially emulate CPython's behavior. See :ref:`oxidized_importer_dunder_init_module_names` for more. (#317) +* The lifetime of ``pyembed::MainPythonInterpreter.acquire_gil()``'s return + value has been adjusted so the Rust compiler will refuse to compile code + that could crash due to attempting to use a finalized interpreter. (#345) New Features ^^^^^^^^^^^^ diff --git a/pyembed/src/interpreter.rs b/pyembed/src/interpreter.rs index a95875e3a..2b72a3655 100644 --- a/pyembed/src/interpreter.rs +++ b/pyembed/src/interpreter.rs @@ -422,7 +422,12 @@ impl<'python, 'interpreter, 'resources> MainPythonInterpreter<'python, 'interpre } /// Ensure the Python GIL is acquired, returning a handle on the interpreter. - pub fn acquire_gil(&mut self) -> Result, &'static str> { + /// + /// The returned value has a lifetime of the `MainPythonInterpreter` + /// instance. This is because `MainPythonInterpreter.drop()` finalizes + /// the interpreter. The borrow checker should refuse to compile code + /// where the returned `Python` outlives `self`. + pub fn acquire_gil(&mut self) -> Result, &'static str> { match self.interpreter_state { InterpreterState::NotStarted => { return Err("interpreter not initialized"); diff --git a/pyembed/src/test/main_python_interpreter.rs b/pyembed/src/test/main_python_interpreter.rs index d182fe647..a1ef16102 100644 --- a/pyembed/src/test/main_python_interpreter.rs +++ b/pyembed/src/test/main_python_interpreter.rs @@ -9,15 +9,12 @@ use { rusty_fork_test! { #[test] - #[should_panic] fn test_instantiate_interpreter() { - let py = { - let mut config = OxidizedPythonInterpreterConfig::default(); - config.interpreter_config.parse_argv = Some(false); - config.set_missing_path_configuration = false; - let mut interp = MainPythonInterpreter::new(config).unwrap(); - interp.acquire_gil().unwrap() - }; + let mut config = OxidizedPythonInterpreterConfig::default(); + config.interpreter_config.parse_argv = Some(false); + config.set_missing_path_configuration = false; + let mut interp = MainPythonInterpreter::new(config).unwrap(); + let py = interp.acquire_gil().unwrap(); py.import("sys").unwrap(); } }