Skip to content

[cpyrt] Unregister proxies whose C++ object was replaced through T** - #161

Open
guitargeek wants to merge 1 commit into
compiler-research:mainfrom
guitargeek:regulator-replaced-object
Open

guitargeek wants to merge 1 commit into
compiler-research:mainfrom
guitargeek:regulator-replaced-object

Conversation

@guitargeek

Copy link
Copy Markdown
Collaborator

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.

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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant