fix possibly leaked borrow in PyRef::into_super#5281
Conversation
Icxolu
left a comment
Icxolu
left a comment
There was a problem hiding this comment.
Clever solution, I also think this should work. I guess this means there is a brief moment where the borrow count is not completely accurate, but I don't think this is observable anywhere (and it would be higher, which is safe)
src/pycell.rs
Outdated
| if <T::Frozen as crate::pyclass::boolean_struct::private::Boolean>::VALUE | ||
| != <U::Frozen as crate::pyclass::boolean_struct::private::Boolean>::VALUE |
There was a problem hiding this comment.
I think we only need to check this when going from mutable to frozen (and not when going from frozen to mutable because that would already use Us borrow checker), so maybe !T::Value && U::Value would be a tiny optimization.
There was a problem hiding this comment.
I think that's correct too, have adjusted accordingly 👍
Agreed, I figure that having the shared borrow count be temporarily raised is not a concern, even if it's a slight inefficiency. It's such a corner case that I doubt its a problem in practice, especially concerned to the extremely subtle and confusing bug users would face if they ran into this. |
davidhewitt
changed the title
2 participants
Builds on top of #5253 with a solution that I think might now be correct 🤔