pyo3 icon indicating copy to clipboard operation
pyo3 copied to clipboard

Trait Bound Documentation without `GILPool`

Open kiranshila opened this issue 1 year ago • 5 comments

I'm attempting to implement a Python interface to some Rust hardware driver code that was written against Rust hardware traits. For the python interface, the implementation of this trait needs to be via calls to Python - but the logic of the driver is, of course, written in Rust. This results in a lot of ping-ponging between Rust and Python, something that seems really hard to get right.

In the docs, there is this page on trait bounds, but it uses the Python::with_gil/GILPool API. I'm still not quite sure how that interacts with the new Bound API - but my issue/question is on how these docs change if we weren't to use the GILPool. i.e, we need to get a Bound into the trait.

I have some code that looks similar to the docs example, but with the instance of the PyObject for which we want to implement a Rust trait for inside another Rust struct that is generic on T which requires the implementation of that trait.

// ------ Foreign Rust Library Code

trait ForeignTrait {
    fn trait_method(&mut self) -> f32;
}

struct ForeignType<T: ForeignTrait> {
    inner: T,
}

impl<T> ForeignType<T>
where
    T: ForeignTrait,
{
    fn do_something(&mut self) -> f32 {
        let x = self.inner.trait_method();
        x * x
    }
}

// ----- My Python Wrapper Code

/// A newtype around an instance of a python class that has a method I want to use from rust
struct PyWrapper(Py<PyAny>);

impl ForeignTrait for PyWrapper {
    fn trait_method(&mut self) -> f32 {
        Python::with_gil(|py| {
            let obj = self.0.bind(py);
            let x: f32 = obj
                .call_method0("fn_that_returns_a_float")
                .unwrap() // We'll do real error handling at some point
                .extract()
                .unwrap();
            x
        })
    }
}

/// A "top-level" struct that I'll expose to Python
/// Basically just a surrogate for `ForeignType` as an FFI-shim
#[pyclass]
struct TopLevel(ForeignType<PyWrapper>);

#[pymethods]
impl TopLevel {
    #[new]
    fn new(obj_from_python: Bound<'_, PyAny>) -> Self {
        Self(ForeignType { inner: PyWrapper(obj_from_python.unbind()) })
    }

    /// A shim for the inner function of ForeignType
    fn do_something(&mut self) -> f32 {
        self.0.do_something()
    }
}

In the real code I've written, I occasionally get RuntimeError: already borrowed - which means something about the setup here breaks some rules. I'm wondering what assumptions in this example are not correct such that I could get that runtime error, if this is related to some weird interaction between the Bound and Python::with_gil trait impl, what kind of solutions there are, and if we could work together on some docs to explain this.

Cheers!

kiranshila avatar Aug 23 '24 18:08 kiranshila

You will get a panic If fn_that_returns_a_float() will call TopLevel::do_something(). The way to fix that is by using Bound<Self> instead of &mut self:

/// A newtype around an instance of a python class that has a method I want to use from rust
struct PyWrapper(Py<PyAny>);

impl ForeignTrait for PyWrapper {
    fn trait_method(&mut self) -> f32 {
        Python::with_gil(|py| {
            let obj = self.0.bind(py);
            let x: f32 = obj
                .call_method0("fn_that_returns_a_float")
                .unwrap() // We'll do real error handling at some point
                .extract()
                .unwrap();
            x
        })
    }
}

/// A "top-level" struct that I'll expose to Python
/// Basically just a surrogate for `ForeignType` as an FFI-shim
#[pyclass]
struct TopLevel(Py<PyAny>);

#[pymethods]
impl TopLevel {
    #[new]
    fn new(obj_from_python: Bound<'_, PyAny>) -> Self {
        Self(obj_from_python.unbind())
    }

    /// A shim for the inner function of ForeignType
    fn do_something(this: Bound<'_, Self>) -> f32 {
        let wrapper = PyWrapper(this.borrow().0.0.clone_ref(this.py()));
        ForeignType { inner: wrapper }.do_something();
    }
}

ChayimFriedman2 avatar Aug 27 '24 01:08 ChayimFriedman2

Thank you for your response! So, this implies that we have to construct a ForeignType on every call to an internal function? How would we manage a situation where that struct contains state?

kiranshila avatar Aug 27 '24 01:08 kiranshila

You can store a ForeignType and clone it instead. Or, if that's not possible, take it and return it back at the end, but that will mean users won't be able to call your method recursively (but they still won't get PanicException).

ChayimFriedman2 avatar Aug 27 '24 02:08 ChayimFriedman2

I think I don't completely understand.

If I need to work through a PyRef how about

#[pyclass]
struct TopLevel(ForeignType<PyWrapper>);

#[pymethods]
impl TopLevel {
    #[new]
    fn new(obj_from_python: Bound<'_, PyAny>) -> Self {
        Self(obj_from_python.unbind())
    }

    /// A shim for the inner function of ForeignType
    fn do_something(this: Bound<'_, Self>) -> f32 {
        this.borrow_mut().0.do_something()
    }
}

kiranshila avatar Aug 27 '24 04:08 kiranshila

I think I don't completely understand.

If I need to work through a PyRef how about

#[pyclass]
struct TopLevel(ForeignType<PyWrapper>);

#[pymethods]
impl TopLevel {
    #[new]
    fn new(obj_from_python: Bound<'_, PyAny>) -> Self {
        Self(obj_from_python.unbind())
    }

    /// A shim for the inner function of ForeignType
    fn do_something(this: Bound<'_, Self>) -> f32 {
        this.borrow_mut().0.do_something()
    }
}

This won't work. PyRef alone isn't enough, you need to not hold the borrow when you call arbitrary Python code.

ChayimFriedman2 avatar Aug 27 '24 05:08 ChayimFriedman2

As discussed above in the thread, the docs are already updated for Bound and the issue was more to do with interior mutability.

Will close here as no further action.

davidhewitt avatar Oct 22 '24 07:10 davidhewitt