[C API] Prepare the C API to make PyThreadState opaque: add getter functions

[vstinner]

BPO	39947
Nosy	@vstinner, @fabioz, @markshannon, @ericsnowcurrently, @iritkatriel
PRs	bpo-39947: Hide implementation detail of trashcan macros #18971 bpo-39947: Move Py_EnterRecursiveCall() to internal C API #18972 bpo-39947: Move get_recursion_depth() to _testinternalcapi #18974 bpo-39947: Use _PyInterpreterState_GET_UNSAFE() #18978 bpo-39947: Add PyInterpreterState_Get() function #18979 bpo-39947: Add PyThreadState_GetInterpreter() #18981 bpo-39947: Add PyThreadState_GetFrame() function #19092 bpo-39947: Use PyThreadState_GetFrame() #19159 bpo-39947: Add _PyThreadState_GetDict() #19160 bpo-39947: Add PyThreadState_GetID() function #19163 Add symbols of the stable ABI to python3dll.c #23598 [3.9] bpo-42415: Add symbols of the stable ABI to python3dll.c (GH-23598) #23801 bpo-39947: revert incorrect change to a comment #26788 bpo-39947: Remove old private trashcan C API functions #26869 bpo-39947: Add PyThreadState_SetTrace() function #29121

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2020-03-12.17:46:54.454>
labels = ['expert-C-API', '3.11']
title = '[C API] Make the PyThreadState structure opaque (move it to the internal C API)'
updated_at = <Date 2022-04-06.13:20:11.746>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2022-04-06.13:20:11.746>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['C API']
creation = <Date 2020-03-12.17:46:54.454>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 39947
keywords = ['patch']
message_count = 25.0
messages = ['364034', '364078', '364080', '364083', '364084', '364103', '364108', '364111', '364113', '364124', '364591', '364674', '365011', '365021', '365022', '370589', '372319', '382551', '383169', '383210', '390054', '396421', '402368', '403816', '416868']
nosy_count = 5.0
nosy_names = ['vstinner', 'fabioz', 'Mark.Shannon', 'eric.snow', 'iritkatriel']
pr_nums = ['18971', '18972', '18974', '18978', '18979', '18981', '19092', '19159', '19160', '19163', '23598', '23801', '26788', '26869', '29121']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue39947'
versions = ['Python 3.11']

[vstinner]

Python 3.8 moved PyInterpreterState to the internal C API (commit be3b295 of bpo-35886)... which caused bpo-38500 issue.

In Python 3.9, I provided Py_EnterRecursiveCall() and Py_LeaveRecursiveCall() as regular functions for the limited API: commit f4b1e3d of bpo-38644. Previously, there were defined as macros, but these macros didn’t compile with the limited C API which cannot access PyThreadState.recursion_depth field (the structure is opaque in the limited C API).

That's an enhancement for the limited C API, but PyThreadState is still exposed to the "cpython" C API (Include/cpython/).

We should prepare the C API to make the PyThreadState structure opaque. It cannot be done at once, there are different consumers of the PyThreadState structure. In CPython code base, I found:

• Py_TRASHCAN_BEGIN_CONDITION and Py_TRASHCAN_END macros access tstate->trash_delete_nesting field. Maybe we can hide these implementation details into new private function calls.

• faulthandler.c: faulthandler_py_enable() reads tstate->interp. We should maybe provide a getter function.

• _tracemalloc.c: traceback_get_frames() reads tstate->frame. We should maybe provide a getter function.

• Private _Py_EnterRecursiveCall() and _Py_LeaveRecursiveCall() access tstate->recursion_depth. One solution is to move these functions to the internal C API.

faulthandler and _tracemalloc are low-level debugging C extensions. Maybe it's ok for them to use the internal C API. But they are examples of C extensions accessing directly the PyThreadState structure.

See also bpo-39946 "Is it time to remove _PyThreadState_GetFrame() hook?" about PyThreadState.frame.

[vstinner]

New changeset 224481a by Victor Stinner in branch 'master':
bpo-39947: Move Py_EnterRecursiveCall() to internal C API (GH-18972)
224481a

[fabioz]

As a note, externally I have to use it in pydevd to set the tracing for different threads -- i.e.: https://bugs.python.org/issue35370

Will that still be possible?

[vstinner]

As a note, externally I have to use it in pydevd to set the tracing for different threads -- i.e.: https://bugs.python.org/issue35370 Will that still be possible?

My intent is not to prevent third-party C extension modules to modify PyThreadState, but to make the structure opaque. I mean that we should add getter and setter function for the most commonly used PyThreadState fields.

[vstinner]

New changeset 3f2f4fe by Victor Stinner in branch 'master':
bpo-39947: Move get_recursion_depth() to _testinternalcapi (GH-18974)
3f2f4fe

[vstinner]

New changeset 38965ec by Victor Stinner in branch 'master':
bpo-39947: Hide implementation detail of trashcan macros (GH-18971)
38965ec

[vstinner]

I tested to build numpy with an opaque PyThreadState. First issue, Plex gets the current interpreter using PyThreadState.interp:

/tmp/pip-install-aq60p8w2/Cython/Cython/Plex/Scanners.c:7447:73: erreur: déréférencement d'un pointeur du type incomplet « PyThreadState »{alias « struct _ts »}
 7447 | PY_INT64_T current_id = PyInterpreterState_GetID(PyThreadState_Get()->interp);

We should add a PyThreadState_GetInterpreter(tstate) getter. faulthandler_py_enable() would use it for example.

Maybe _PyInterpreterState_Get() can be used, but it's a private function. There are also _PyThreadState_UncheckedGet() and _PyGILState_GetInterpreterStateUnsafe() which are worse: don't check for NULL pointers.

[vstinner]

New changeset ff4584c by Victor Stinner in branch 'master':
bpo-39947: Use _PyInterpreterState_GET_UNSAFE() (GH-18978)
ff4584c

[vstinner]

New changeset be79373 by Victor Stinner in branch 'master':
bpo-39947: Add PyInterpreterState_Get() function (GH-18979)
be79373