avoid creating PyRef inside __traverse__ handler

[davidhewitt]

See #4265 (comment)

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.

[davidhewitt]

A bit of a drive-by optimization, to add a niche for the 0.

[davidhewitt]

I completely removed the Python::assume_gil_acquired call from _call_traverse, so I needed to place a different reference back in for the lifetime. This is sufficient because the *mut c_void already prevents Send/Sync.

[davidhewitt]

cc @ngoldbaum

[mejrs]

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.

[Icxolu]

I believe that would need #4390. Also in that thread #4390 (comment) there was the suggestion to maybe delay that change to 0.24

[davidhewitt]

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.

[ngoldbaum]

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.

[davidhewitt]

Great, will merge this. I am considering landing this as part of a stack of fixes in a 0.22.3 patch.