Skip to content

Commit

Permalink
fix: make sure always have GIL when creating subinterpreter
Browse files Browse the repository at this point in the history
  • Loading branch information
brownben committed Nov 8, 2024
1 parent f0c4819 commit f75255c
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 7 deletions.
5 changes: 3 additions & 2 deletions src/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::collections::{BTreeSet, HashMap};
use std::ffi::CString;
use std::{fs, path};

use crate::python::{self, PyObject};
use crate::python::{self, Interpreter, PyObject};

/// Enables line coverage collection, and returns the tracer object
pub fn enable_collection() -> PyObject {
Expand Down Expand Up @@ -103,6 +103,7 @@ impl FromIterator<(String, BTreeSet<i32>)> for Lines {
/// not be reported but are still executed. So we need to find which lines could be run,
/// rather than just the range of the file.
pub fn get_executable_lines(
interpreter: &Interpreter,
coverage_include: &[path::PathBuf],
coverage_exclude: &[path::PathBuf],
) -> Lines {
Expand Down Expand Up @@ -147,7 +148,7 @@ pub fn get_executable_lines(

// as we are compiling python, we need an interpreter
let line_numbers =
python::SubInterpreter::new().run(|| get_line_numbers_for_python_file(&path));
python::SubInterpreter::new(interpreter).run(|| get_line_numbers_for_python_file(&path));

if line_numbers.is_empty() {
None
Expand Down
7 changes: 4 additions & 3 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,14 @@ fn main() -> ExitCode {
reporter.discovered(&discovered);

// Main Python interpreter must be initialized in the main thread
let _interpreter = python::Interpreter::initialize();
let interpreter = python::Interpreter::initialize();

// Run tests
let results: TestSummary = discovered
.tests
.par_iter()
.map(|test| {
let mut subinterpreter = python::SubInterpreter::new();
let mut subinterpreter = python::SubInterpreter::new(&interpreter);

if args.coverage.enabled {
subinterpreter.enable_coverage();
Expand Down Expand Up @@ -131,7 +131,8 @@ fn main() -> ExitCode {
&args.coverage.exclude
};

let possible_lines = coverage::get_executable_lines(coverage_include, coverage_exclude);
let possible_lines =
coverage::get_executable_lines(&interpreter, coverage_include, coverage_exclude);
coverage::print_summary(&possible_lines, &results.executed_lines);
}

Expand Down
6 changes: 4 additions & 2 deletions src/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ impl Drop for Interpreter {
unsafe { ffi::Py_FinalizeEx() };
}
}
unsafe impl Sync for Interpreter {}

/// Represents a Python Subinterpreter (An interpreter for a specific thread)
pub struct SubInterpreter {
Expand All @@ -84,12 +85,13 @@ impl SubInterpreter {
/// Creates a new subinterpreter
///
/// SAFETY: The main interpreter must have been initialized
pub fn new() -> Self {
pub fn new(main_interpreter: &Interpreter) -> Self {
let mut interpreter_state = std::ptr::null_mut();

unsafe {
ffi::PyEval_RestoreThread(main_interpreter.main_thread_state); // Get the GIL
ffi::Py_NewInterpreterFromConfig(&mut interpreter_state, &Self::DEFAULT_CONFIG);
ffi::PyEval_SaveThread(); // Stops Deadlock
ffi::PyEval_SaveThread(); // Release the GIL
};

Self {
Expand Down

0 comments on commit f75255c

Please sign in to comment.