classification
Title: [C API] Make the PyThreadState structure opaque (move it to the internal C API)
Type: Stage: patch review
Components: C API Versions: Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Mark.Shannon, eric.snow, fabioz, vstinner
Priority: normal Keywords: patch

Created on 2020-03-12 17:46 by vstinner, last changed 2020-06-02 02:04 by vstinner.

Pull Requests
URL Status Linked Edit
PR 18971 merged vstinner, 2020-03-12 23:05
PR 18972 merged vstinner, 2020-03-12 23:17
PR 18974 merged vstinner, 2020-03-13 09:32
PR 18978 merged vstinner, 2020-03-13 16:34
PR 18979 merged vstinner, 2020-03-13 16:42
PR 18981 merged vstinner, 2020-03-13 17:12
PR 19092 merged vstinner, 2020-03-20 13:23
PR 19159 merged vstinner, 2020-03-25 18:29
PR 19160 merged vstinner, 2020-03-25 18:39
PR 19163 merged vstinner, 2020-03-25 18:47
Messages (16)
msg364034 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-12 17:46
Python 3.8 moved PyInterpreterState to the internal C API (commit be3b295838547bba267eb08434b418ef0df87ee0 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 f4b1e3d7c64985f5d5b00f6cc9a1c146bbbfd613 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.
msg364078 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-13 09:20
New changeset 224481a8c988fca12f488544edd2f01c0af2a91d by Victor Stinner in branch 'master':
bpo-39947: Move Py_EnterRecursiveCall() to internal C API (GH-18972)
https://github.com/python/cpython/commit/224481a8c988fca12f488544edd2f01c0af2a91d
msg364080 - (view) Author: Fabio Zadrozny (fabioz) Date: 2020-03-13 11:39
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?
msg364083 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-13 12:06
> 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.
msg364084 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-13 12:07
New changeset 3f2f4fefca388bc62fc2a7d07cb6ef24197c84bd by Victor Stinner in branch 'master':
bpo-39947: Move get_recursion_depth() to _testinternalcapi (GH-18974)
https://github.com/python/cpython/commit/3f2f4fefca388bc62fc2a7d07cb6ef24197c84bd
msg364103 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-13 15:51
New changeset 38965ec5411da60d312b59be281f3510d58e0cf1 by Victor Stinner in branch 'master':
bpo-39947: Hide implementation detail of trashcan macros (GH-18971)
https://github.com/python/cpython/commit/38965ec5411da60d312b59be281f3510d58e0cf1
msg364108 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-13 16:08
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.
msg364111 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-13 17:04
New changeset ff4584caca04cb3da0dbd5b1e9bf67e40adf5312 by Victor Stinner in branch 'master':
bpo-39947: Use _PyInterpreterState_GET_UNSAFE() (GH-18978)
https://github.com/python/cpython/commit/ff4584caca04cb3da0dbd5b1e9bf67e40adf5312
msg364113 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-13 17:15
New changeset be79373a78c0d75fc715ab64253c9b757987a848 by Victor Stinner in branch 'master':
bpo-39947: Add PyInterpreterState_Get() function (GH-18979)
https://github.com/python/cpython/commit/be79373a78c0d75fc715ab64253c9b757987a848
msg364124 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-13 22:38
New changeset 8fb02b6e1942811c8d81041e7df3f5f1f4b1d410 by Victor Stinner in branch 'master':
bpo-39947: Add PyThreadState_GetInterpreter() (GH-18981)
https://github.com/python/cpython/commit/8fb02b6e1942811c8d81041e7df3f5f1f4b1d410
msg364591 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-19 03:13
I added PyInterpreterState_Get() and PyThreadState_GetInterpreter() functions to get the interpreter.
msg364674 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-20 14:51
New changeset fd1e1a18fa3befe5b6eeac32e0561e15c7e5164b by Victor Stinner in branch 'master':
bpo-39947: Add PyThreadState_GetFrame() function (GH-19092)
https://github.com/python/cpython/commit/fd1e1a18fa3befe5b6eeac32e0561e15c7e5164b
msg365011 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-25 18:52
New changeset 3072338642156a5edfa2deef2557288fff73c22a by Victor Stinner in branch 'master':
bpo-39947: Use PyThreadState_GetFrame() (GH-19159)
https://github.com/python/cpython/commit/3072338642156a5edfa2deef2557288fff73c22a
msg365021 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-25 20:23
New changeset 0e427c6d159e86f17270770cd8dc37372e3c4004 by Victor Stinner in branch 'master':
bpo-39947: Add _PyThreadState_GetDict() function (GH-19160)
https://github.com/python/cpython/commit/0e427c6d159e86f17270770cd8dc37372e3c4004
msg365022 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-25 20:23
New changeset 5c3cda0d1a850a1a9b43892f48376b8876bd5863 by Victor Stinner in branch 'master':
bpo-39947: Add PyThreadState_GetID() function (GH-19163)
https://github.com/python/cpython/commit/5c3cda0d1a850a1a9b43892f48376b8876bd5863
msg370589 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-06-02 02:04
Cython still access multiple PyThreadState members which have no getter or setter yet.

__Pyx_PyErr_ExceptionMatchesInState():

    PyObject *exc_type = tstate->curexc_type;
    ...

=> internal _PyErr_Occurred(tstate) could solve this issue: move it the public/private API? Or expose internal _PyErr_ExceptionMatches(tstate, exc)?

__Pyx_ErrRestoreInState() is a reimplementation of internal _PyErr_Restore(): get/set curexc_type, curexc_value and curexc_traceback members.

__Pyx_PyFunction_FastCallNoKw:

static PyObject* __Pyx_PyFunction_FastCallNoKw(...) {
    ...
    ++tstate->recursion_depth;
    Py_DECREF(f);
    --tstate->recursion_depth;
    return result;
}

Why not calling Py_EnterRecursiveCall/Py_LeaveRecursiveCall?

There are likely others.
History
Date User Action Args
2020-06-02 02:04:06vstinnersetmessages: + msg370589
2020-04-14 14:01:21vstinnersettitle: Make the PyThreadState structure opaque (move it to the internal C API) -> [C API] Make the PyThreadState structure opaque (move it to the internal C API)
2020-03-25 20:23:56vstinnersetmessages: + msg365022
2020-03-25 20:23:03vstinnersetmessages: + msg365021
2020-03-25 18:52:08vstinnersetmessages: + msg365011
2020-03-25 18:47:40vstinnersetpull_requests: + pull_request18523
2020-03-25 18:39:25vstinnersetpull_requests: + pull_request18520
2020-03-25 18:29:50vstinnersetpull_requests: + pull_request18519
2020-03-20 14:51:52vstinnersetmessages: + msg364674
2020-03-20 13:23:44vstinnersetpull_requests: + pull_request18452
2020-03-19 03:13:40vstinnersetmessages: + msg364591
2020-03-13 22:38:11vstinnersetmessages: + msg364124
2020-03-13 17:15:37vstinnersetmessages: + msg364113
2020-03-13 17:12:33vstinnersetpull_requests: + pull_request18329
2020-03-13 17:04:00vstinnersetmessages: + msg364111
2020-03-13 16:42:16vstinnersetpull_requests: + pull_request18327
2020-03-13 16:34:14vstinnersetpull_requests: + pull_request18326
2020-03-13 16:08:24vstinnersetmessages: + msg364108
2020-03-13 15:51:55vstinnersetmessages: + msg364103
2020-03-13 12:07:54vstinnersetmessages: + msg364084
2020-03-13 12:06:52vstinnersetmessages: + msg364083
2020-03-13 11:39:43fabiozsetnosy: + fabioz
messages: + msg364080
2020-03-13 09:32:03vstinnersetpull_requests: + pull_request18323
2020-03-13 09:20:03vstinnersetmessages: + msg364078
2020-03-12 23:17:53vstinnersetpull_requests: + pull_request18320
2020-03-12 23:05:18vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request18319
2020-03-12 17:47:27vstinnersetnosy: + Mark.Shannon, eric.snow

title: Move PyThreadState structure to the internal C API -> Make the PyThreadState structure opaque (move it to the internal C API)
2020-03-12 17:46:54vstinnercreate