avoid creating `PyRef` inside `__traverse__` handler
See https://github.com/PyO3/pyo3/issues/4265#issuecomment-2305890021
We cannot use PyRef<T> to guard access to the value inside a __traverse__ handler. Instead I use the PyClassObject<T> and manually perform the borrow checking.
cc @ngoldbaum
We could also make the change suggested by the comment here....
#[repr(transparent)]
pub struct PyRef<'p, T: PyClass> {
// TODO: once the GIL Ref API is removed, consider adding a lifetime parameter to `PyRef` to
// store `Borrowed` here instead, avoiding reference counting overhead.
inner: Bound<'p, T>,
}
as that would also avoid changing reference counts.
I believe that would need #4390. Also in that thread https://github.com/PyO3/pyo3/pull/4390#discussion_r1702907653 there was the suggestion to maybe delay that change to 0.24
Yes agreed, I considered changing PyRef but would prefer to delay given there's enough breaking changes coming in 0.23. I also want to be careful with changing PyRef right now because I think it might need other changes for the freethreaded build.
I also think I might backport this fix to 0.22, which would be another reason not to have the breaking change here.
I did some local testing on this PR.
After rebasing on main to fix some noisy tests that were fixed by me last week, this does seem to fix the "accessing the GIL while a __traverse__ implementation is running" panics.
Great, will merge this. I am considering landing this as part of a stack of fixes in a 0.22.3 patch.