classification
Title: Add PyInterpreterState.runtime.
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.snow Nosy List: eric.snow, vstinner
Priority: normal Keywords: patch

Created on 2019-05-06 18:45 by eric.snow, last changed 2020-03-09 23:28 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 13129 merged eric.snow, 2019-05-06 18:47
PR 13795 merged vstinner, 2019-06-04 00:59
Messages (9)
msg341595 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-05-06 18:45
Currently we use the _PyRuntime static global to access the runtime state in various places.  At the same time, in thread contexts we get access to the thread state from Thread-Local Storage and the interpreter state by indirection from there.  We should do the same for the runtime state instead of using the global directly.

My plan is to add a PyInterpreterState.runtime field.  It can then be used in the same way we use PyThreadState.interp to access the interpreter state.
msg342043 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-05-10 02:20
I don't see the point of PyInterpreterState.runtime:

* I understood that we should have and only one _PyRuntimeState instance: _PyRuntime. Why not accessing it directly?
* I understood that most _PyRuntimeState fields should be moved to PyInterpreterState. Once this refactoring will be done, we will almost never have to access _PyRuntime.
msg342134 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-05-10 21:22
Eric is working on a different approach: https://bugs.python.org/issue36854
msg344142 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-06-01 03:16
New changeset 396e0a8d9dc65453cb9d53500d0a620602656cfe by Eric Snow in branch 'master':
bpo-36818: Add PyInterpreterState.runtime field. (gh-13129)
https://github.com/python/cpython/commit/396e0a8d9dc65453cb9d53500d0a620602656cfe
msg344513 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-04 01:15
New changeset 0fd2c300c2a85b3b227a907b2a39ef79fa86d850 by Victor Stinner in branch 'master':
Revert "bpo-36818: Add PyInterpreterState.runtime field. (gh-13129)" (GH-13795)
https://github.com/python/cpython/commit/0fd2c300c2a85b3b227a907b2a39ef79fa86d850
msg344514 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-04 01:19
This change introduced a regression:
https://bugs.python.org/issue37135#msg344511

I had to revert the change to fix Python for 3.8 beta1 release.
msg344515 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-04 01:21
By the way, I have questions about the rationale of this change.

> Currently we use the _PyRuntime static global to access the runtime state in various places.  At the same time, in thread contexts we get access to the thread state from Thread-Local Storage and the interpreter state by indirection from there.  We should do the same for the runtime state instead of using the global directly.

Right now I'm confused. _PyRuntimeState contains scatted states: things which should be moved to PyInterpreterState or even PyThreadState, and things which should remain in _PyRuntimeState. My notes on this topic:
https://pythoncapi.readthedocs.io/runtime.html

Is PyInterpreterState.runtime a temporary fix until things are moved?

Or do you plan to never access _PyRuntime directly, and always through interp->runtime?

It seems like in the short term, we must continue to access "_PyRuntime" through a separated "runtime" pointer passed to functions, to handle daemon threads during Python shutdown.
msg344558 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-04 12:46
See https://bugs.python.org/issue37135#msg344509 to reproduce https://bugs.python.org/issue37135 crash without multiprocessing: with this procedure, I reproduce the crash immediately on Linux. No need for a magic super slow buildbot.
msg363789 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-09 23:28
I added PyInterpreterState.runtime in bpo-36710:

commit 01b1cc12e7c6a3d6a3d27ba7c731687d57aae92a
Author: Victor Stinner <vstinner@python.org>
Date:   Wed Nov 20 02:27:56 2019 +0100

    bpo-36710: Add PyInterpreterState.runtime field (GH-17270)
    
    Add PyInterpreterState.runtime field: reference to the _PyRuntime
    global variable. This field exists to not have to pass runtime in
    addition to tstate to a function.  Get runtime from tstate:
    tstate->interp->runtime.
    
    Remove "_PyRuntimeState *runtime" parameter from functions already
    taking a "PyThreadState *tstate" parameter.
    
    _PyGC_Init() first parameter becomes "PyThreadState *tstate".

--

> This change introduced a regression:
> https://bugs.python.org/issue37135#msg344511

Funny/not funny, I introduced the same bug and I fixed it in bpo-39877.
History
Date User Action Args
2020-03-09 23:28:59vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg363789
2019-06-04 12:46:06vstinnersetmessages: + msg344558
2019-06-04 01:21:32vstinnersetmessages: + msg344515
2019-06-04 01:19:05vstinnersetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg344514
2019-06-04 01:15:11vstinnersetmessages: + msg344513
2019-06-04 00:59:06vstinnersetpull_requests: + pull_request13680
2019-06-01 04:30:17eric.snowsetresolution: rejected -> fixed
2019-06-01 03:16:52eric.snowsetmessages: + msg344142
2019-05-10 21:22:11vstinnersetstatus: open -> closed
resolution: rejected
messages: + msg342134

stage: patch review -> resolved
2019-05-10 02:20:18vstinnersetmessages: + msg342043
2019-05-06 18:47:38eric.snowsetkeywords: + patch
pull_requests: + pull_request13042
2019-05-06 18:45:44eric.snowcreate