PyO3 / pyo3

Rust bindings for the Python interpreter
https://pyo3.rs
Apache License 2.0
12.39k stars 765 forks source link

`Clone` (or alternative) on `GILOnceCell` #4428

Closed jakelishman closed 2 months ago

jakelishman commented 3 months ago

I have a pyclass struct that wants to contain a GILOnceCell<Py<PyAny>> for caching of a lazily constructed Python object, where the initialiser yields control to the Python interpreter so runs the risk of re-entrancy problems with regular OnceCell.

I'd originally tried to derive Clone on my struct (I'm still using py-clone for the time being, but working to disentangle our data structures), which if I understand correctly must fail for GILOnceCell because it requires the GIL to mediate both gets and sets. For the time being, I think the correct course of action for me for the clone is to manually GILOnceCell::new(other_cell.get(py).map(|ob| ob.clone_ref(py))) (or similar).

edit: if there's an A/B problem here, and there's something better I could be doing with this kind of cache, I'm totally open to alternatives to the answer being "that's not the right way to do this".

Would it be possible to either:

davidhewitt commented 3 months ago

This is a great question. The clone_ref function you propose seems relatively fine to add, to me.

A related question is what to do about GILOnceCell in its entirety when it no longer makes sense in freethreaded Python (see #4265). It would be nice to find a solution that works for both GIL and freethreaded builds. Does that mean removing this type and replacing it with different semantics? Hopefully we will have an answer soon...

jakelishman commented 2 months ago

Thanks for the response, and sorry for disappearing for a long while there! I made #4511 with the top bullet, but happy to change it however needed.

For GILOnceCell in free-threaded Python: naively to me, it seems like GILOnceCell simply can't exist (or at least can't be Send) in a free-threaded build because the GIL no longer does the mediation the type relies on. I know we're intending to find a new solution to holding shared Rust/Python data in our library (Qiskit) when we start worrying about supporting free-threading, but it's just one on a fairly large pile of things preventing us from enabling GIL-free Python-space threading, so for now I'm sticking my head in the sand! We haven't even managed to move off py-clone yet, and that's going to be enough of an ordeal (of our own making) for us.

davidhewitt commented 2 months ago

I think for py-clone you can get a long way by replacing PyObject with Arc<PyObject>>, as that gives you the clone-without-the-GIL back. Maybe this should be in the migration guide...

davidhewitt commented 2 months ago

For freethreaded GILOnceCell, I'd welcome any opinions on #4512 and #4513