Repository navigation
[cpyrt] Unregister proxies whose C++ object was replaced through T** - #161
Open
guitargeek wants to merge 1 commit into
Open
guitargeek wants to merge 1 commit into
guitargeek wants to merge 1 commit into
Conversation
When a C++ function takes a proxied object as `T**` (or `void**`), the converter passes the address of the proxy's own pointer slot, so the function can delete the object and store a new one in the proxy, as `CppjitTestData::set_pod_ptrptr_out` does in test_datatypes. The memory regulator still tracks the proxy under the old address, and when the proxy dies, `UnregisterPyObject` looks it up under the new one. That erases nothing, or erases the entry of an unrelated proxy that is tracked under the new address, and leaves the old entry pointing at the dead proxy. Once the allocator hands the old address to a new object of the same class, `RegisterPyObject` finds the dangling entry and clears `kIsRegulated` in the `fFlags` of the dead proxy, i.e. it clears bit 0x800 somewhere in whatever object reused that memory. This was the sporadic crash of test_datatypes on the mac-beta runner of ROOT CI (root-project/root#23740): the constructor call in test21 got the address of the pod that test16 had replaced, and the write landed in the item array of pytest's list of passed reports, which the garbage collector later traversed. Only macOS's allocator reused the address at that point. `UnregisterPyObject` now erases an entry only if it belongs to the proxy, and searches the map for the proxy otherwise, which also covers proxies whose held pointer was nulled. The search runs only when a registered proxy is not found under the address it holds now.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a C++ function takes a proxied object as
T**(orvoid**), the converter passes the address of the proxy's own pointer slot, so the function can delete the object and store a new one in the proxy, asCppjitTestData::set_pod_ptrptr_outdoes in test_datatypes. The memory regulator still tracks the proxy under the old address, and when the proxy dies,UnregisterPyObjectlooks it up under the new one. That erases nothing, or erases the entry of an unrelated proxy that is tracked under the new address, and leaves the old entry pointing at the dead proxy.Once the allocator hands the old address to a new object of the same class,
RegisterPyObjectfinds the dangling entry and clearskIsRegulatedin thefFlagsof the dead proxy, i.e. it clears bit 0x800 somewhere in whatever object reused that memory. This was the sporadic crash of test_datatypes on the mac-beta runner of ROOT CI (root-project/root#23740): the constructor call in test21 got the address of the pod that test16 had replaced, and the write landed in the item array of pytest's list of passed reports, which the garbage collector later traversed. Only macOS's allocator reused the address at that point.UnregisterPyObjectnow erases an entry only if it belongs to the proxy, and searches the map for the proxy otherwise, which also covers proxies whose held pointer was nulled. The search runs only when a registered proxy is not found under the address it holds now.