Skip to content

[3.12] gh-110310: Add a Per-Interpreter XID Registry for Heap Types (gh-110311) #110714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Oct 11, 2023

We do the following:

  • add a per-interpreter XID registry (PyInterpreterState.xidregistry)
  • put heap types there (keep static types in _PyRuntimeState.xidregistry)
  • clear the registries during interpreter/runtime finalization
  • avoid duplicate entries in the registry (when _PyCrossInterpreterData_RegisterClass() is called more than once for a type)
  • use Py_TYPE() instead of PyObject_Type() in _PyCrossInterpreterData_Lookup()

The per-interpreter registry helps preserve isolation between interpreters. This is important when heap types are registered, which is something we haven't been doing yet but I will likely do soon.

(cherry-picked from commit 80dc39e)

…ypes (pythongh-110311)

We do the following:

* add a per-interpreter XID registry (PyInterpreterState.xidregistry)
* put heap types there (keep static types in _PyRuntimeState.xidregistry)
* clear the registries during interpreter/runtime finalization
* avoid duplicate entries in the registry (when _PyCrossInterpreterData_RegisterClass() is called more than once for a type)
* use Py_TYPE() instead of PyObject_Type() in _PyCrossInterpreterData_Lookup()

The per-interpreter registry helps preserve isolation between interpreters.  This is important when heap types are registered, which is something we haven't been doing yet but I will likely do soon.

(cherry-picked from commit 80dc39e)
@Yhg1s
Copy link
Member

Yhg1s commented Nov 20, 2023

Can you elaborate (in this PR or in a news item) what this actually fixes? The PR description says it's important for a future change, which makes me wonder if the future change is reasonable for 3.12 and whether this is actually necessary.

@ericsnowcurrently
Copy link
Member Author

but I will likely do soon.

With that I was referring specifically to the _xxinterpchannels.ChannelID type and the test.support.interpreters.*Channel types. Registering them without this patch breaks interpreter isolation, which causes crashes. Furthermore, anyone that registers a heap type via the C-API faces the same problem.

@ericsnowcurrently ericsnowcurrently enabled auto-merge (squash) November 28, 2023 02:03
@ericsnowcurrently ericsnowcurrently merged commit 1e1a30f into python:3.12 Nov 28, 2023
@ericsnowcurrently ericsnowcurrently deleted the backport-80dc39e-fix-xid-registry branch November 28, 2023 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants