Python::with_gil causes a deadlock (multithreaded environment)

[AnatolyBuga]

Question

I am executing python code(below). I pass a python lambda to a Rust function which then executes it in a multithreaded way (polars is basically calling it from inside .par_iter() )

As soon as I introduce Python::with_gil(move |_| {}); I get a deadlock.
Am I just not supposed to call Python::with_gil(move |_| {}); here?

I found this: #3045 which suggests allow_threads but since I am calling a python function I don't think it's for my usecase

Any thoughts would be much appreciated. Somehow exactly the same set up works in py-polars (see map_mul https://github.com/pola-rs/polars/blob/master/py-polars/src/lazy/apply.rs)

Reproducible example

use polars::lazy::dsl::{map_binary, GetOutput};
use polars::prelude::{DataFrame};
use polars::prelude::*;
use pyo3::{pyclass, pymodule, pymethods, Python, types::{PyModule, PyType}, PyResult, PyObject};
use pyo3::prelude::*;

#[pyclass]
pub struct DataSet {
    data: DataFrame
}
#[pymethods]
impl DataSet {
    #[classmethod]
    fn new(_: &PyType) -> Self {

        let data = df!["a" => [1, 2, 3],
         "b" => [4, 5, 6], 
         "c" => ["a", "d", "b"]].unwrap();
        Self{data}
    }

    fn execute(&self, e: &ExprWrapper)  {
        let res = self.data.clone().lazy().with_column(e.e.clone())
            .collect()
            .unwrap();
        dbg!(res);
    }
}

#[pyclass]
pub struct ExprWrapper {
    pub e: Expr
}

#[pyfunction]
fn ultimeasure(pyfnc: PyObject) -> ExprWrapper{
    let functor = move |_: Series, _: Series| { 
        let _ = pyfnc.clone();

        // Here I would convert Series to Python Side Series, call lambda, then result back to Rust Series
        Python::with_gil(move |_| {});
        let srs = Series::new("a", [1,2]);
        Ok(Some(srs))
        };

    let e = map_binary(col("a"), col("b"), functor, GetOutput::from_type(DataType::Float64));
    ExprWrapper{e}
}
#[pymodule]
fn pyo3issue(_py: Python, m: &PyModule) -> PyResult<()> {
    m.add_class::<DataSet>()?;
    m.add_wrapped(wrap_pyfunction!(ultimeasure)).unwrap();
    Ok(())
}

!!! Python side, the code I am executing:

import pyo3issue
import polars as pl

def calculator(a: pl.Series, b: pl.Series):
            print("here")
            return a**b

c = pyo3issue.DataSet.new()
m = pyo3issue.ultimeasure(lambda a,b : a**b)
print(c.execute(m))

Installed versions
Finally, Cargo.toml:

[lib]
name = "pyo3issue"
# "cdylib" is necessary to produce a shared library for Python to import from.
crate-type = ["cdylib"]

[dependencies]
polars = { version = "0.28.0", features = [
    "lazy"
] }
pyo3-polars = "0.2.0"
polars-arrow = "0.28.0"

[dependencies.pyo3]
version = "0.18.0"
# "abi3-py37" tells pyo3 (and maturin) to build using the stable ABI with minimum Python version 3.7
features = ["abi3-py37"]

[adamreichold]

I think the issue is that ultimeasure is already holding the GIL when it is called (as every #[pyfunction] does) and hence you need to surround the call of map_binary by allow_threads.

[adamreichold]

Any thoughts would be much appreciated. Somehow exactly the same set up works in py-polars (see map_mul https://github.com/pola-rs/polars/blob/master/py-polars/src/lazy/apply.rs)

I have to admit to not knowing the difference between map_binary used here and map_multiple used there, but I could image that if all processing is single-threaded, then this would work as the GIL is recursive, i.e. Python::with_gil on a thread which already holds the GIL is basically a no-op.

[AnatolyBuga]

@adamreichold thanks alot! You mean like this:

#[pyfunction]
fn ultimeasure(py: Python, pyobj: PyObject) -> ExprWrapper{

    let functor = move |s1: Series, s2: Series| { 
        let pyobj = pyobj.clone();
        Python::with_gil(move |py| {

        // Here I would convert Series to Python Side Series, call lambda, then result back to Rust Series like this:
        //let s1 = rust_series_to_py_series(py, &s1).unwrap();
        //let s2 = rust_series_to_py_series(py, &s2).unwrap();

        // call the lambda and get a python side Series wrapper
        //let res = match pyobj.call(py, (s1, s2), None) {
        //    Ok(pyobj) => pyobj,
        //    Err(e) => panic!("python apply failed: {}", e.value(py)),
        //};
//
        //let srs = py_series_to_rust_series(res.as_ref(py)).unwrap();
        //Ok(Some(srs))
        let srs = Series::new("a", [1,2]);
        Ok(Some(srs))
        })
    };
    let e = py.allow_threads(||{ 
        map_binary(col("a"), col("b"), functor, GetOutput::from_type(DataType::Float64))
    });
    ExprWrapper{e}
}

This unfortunately is still a deadlock. The thing is functor,which is wrapped in the Expr returned by map_binary here, is executed only when I call .collect() in execute(), so not straight away, but much later.

...and therefore if I try to use ultimeasure's py then I get Python can no be shared...since functor has to be Send
:/

[adamreichold]

is executed only when I call .collect() in execute(), so not straight away, but much later.

Then the allow_threads call needs to be wrapped around that .collect() call as execute is also already holding the GIL due to being a #[pymethod].

[AnatolyBuga]

Then the allow_threads call needs to be wrapped around that .collect() call as execute is also already holding the GIL due to being a #[pymethod].

@adamreichold , this worked! omg, thank you so much! tremendously appreciate!

I also checked polars, and I now see that it also uses allow_threads. So that is how map_mul works.

just out of huge nooby curiousity - by wrapping .execute in py.allow_threads we drop the GIL(the one which was aquired by the main program ie python code) . We then regain the GIL whenever we hit functor , and it seems we are ok that .execute internally uses multiple threads, which may call functor at the same time. Right?

And despite .allow_threads, the actual compute , ie python's calculator would not happen in parallel. Right?

Many thanks

[adamreichold]

Yes, the GIL is a lock which is defined by being held by only one thread of execution at a time. Hence the closure passed to Python::with_gil will not be executed in parallel across multiple threads even if multiple threads do call into functor concurrently. As you noticed, this implies that the actual amount of parallelism and hence speed up is somewhat limited as only one thread will be able to execute the calculator Python code at any time.

[AnatolyBuga]

@adamreichold , fab, thanks alot for this!