Skip to content

Pass _PyRuntimeState as an argument rather than using the _PyRuntime global variable #80891

Description

@vstinner
BPO 36710
Nosy @nascheme, @ncoghlan, @vstinner, @phsilva, @markshannon, @ericsnowcurrently, @jdemeyer, @zooba
PRs
  • bpo-36710: Add 'ceval' local variable to ceval.c #12934
  • bpo-36710: Add runtime parameter to _PyThreadState_Init() #12935
  • bpo-36710: PyOS_AfterFork_Child() pass runtime parameter #12936
  • bpo-36710: Add runtime variable to Py_FinalizeEx() #12937
  • bpo-36710: Add runtime variable to Py_InitializeEx() #12939
  • bpo-36710: Add runtime variable in pystate.c #12956
  • bpo-36710: Add runtime parameter in gcmodule.c #12958
  • bpo-36710: Fix compiler warning on PyThreadState_Delete() #12962
  • bpo-36710: add tstate parameter in errors.c #13540
  • bpo-36710: Add tstate parameter in ceval.c #13547
  • bpo-36710: Pass explicitly tstate in sysmodule.c #14060
  • bpo-36710: Add tstate parameter in import.c #14218
  • bpo-36710: Remove PyImport_Cleanup() function #14221
  • bpo-36710: Use tstate in pylifecycle.c #14249
  • bpo-36710: Add tstate parameter in _warnings.c #14250
  • bpo-36710: Pass tstate parameter to GC collect() #17267
  • bpo-36710: Add PyInterpreterState.runtime field #17270
  • bpo-36710: Pass tstate explicitly in abstract.c #21075
  • 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 = <Date 2020-01-13.16:08:27.587>
    created_at = <Date 2019-04-24.12:58:00.763>
    labels = ['interpreter-core', '3.8']
    title = 'Pass _PyRuntimeState as an argument rather than using the _PyRuntime global variable'
    updated_at = <Date 2020-06-23.13:55:15.412>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2020-06-23.13:55:15.412>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-01-13.16:08:27.587>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2019-04-24.12:58:00.763>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 36710
    keywords = ['patch']
    message_count = 40.0
    messages = ['340772', '340775', '340779', '340784', '340789', '340842', '340845', '340860', '340871', '340873', '340880', '340915', '340930', '340941', '340942', '340945', '340946', '340971', '340986', '340996', '341305', '341500', '342138', '343380', '343393', '345537', '345538', '346016', '346030', '346088', '356496', '357013', '357017', '359915', '360210', '364434', '364438', '364440', '364444', '372170']
    nosy_count = 8.0
    nosy_names = ['nascheme', 'ncoghlan', 'vstinner', 'phsilva', 'Mark.Shannon', 'eric.snow', 'jdemeyer', 'steve.dower']
    pr_nums = ['12934', '12935', '12936', '12937', '12939', '12956', '12958', '12962', '13540', '13547', '14060', '14218', '14221', '14249', '14250', '17267', '17270', '21075']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue36710'
    versions = ['Python 3.8']

    Activity

    1. vstinner commented on Apr 24, 2019

      @vstinner
      MemberAuthor

      Eric Snow moved global variables into a _PyRuntimeState structure which is made of sub-structures. There is a single instance of _PyRuntimeState: the _PyRuntime global variable.

      I would like to add "_PyRuntimeState *" parameters to functions to avoid relying directly on _PyRuntime global variable. The long term goal is to have "stateless" code: don't rely on global variables, only on input parameters. In practice, we will continue to use thread local storage (TLS) to get the "current context" like the current interpreter and the current Python thread state.

    2. vstinner commented on Apr 24, 2019

      @vstinner
      MemberAuthor

      New changeset 8bb3230 by Victor Stinner in branch 'master':
      bpo-36710: Add runtime parameter to _PyThreadState_Init() (GH-12935)
      8bb3230

    3. vstinner commented on Apr 24, 2019

      @vstinner
      MemberAuthor

      New changeset b930a2d by Victor Stinner in branch 'master':
      bpo-36710: PyOS_AfterFork_Child() pass runtime parameter (GH-12936)
      b930a2d

    4. vstinner commented on Apr 24, 2019

      @vstinner
      MemberAuthor

      New changeset 8e91c24 by Victor Stinner in branch 'master':
      bpo-36710: Add runtime variable to Py_FinalizeEx() (GH-12937)
      8e91c24

    5. vstinner commented on Apr 24, 2019

      @vstinner
      MemberAuthor

      New changeset 4312522 by Victor Stinner in branch 'master':
      bpo-36710: Add runtime variable to Py_InitializeEx() (GH-12939)
      4312522

    6. jdemeyer commented on Apr 25, 2019

      @jdemeyer
      Contributor

      I don't really understand the rationale for these changes. What's wrong with the global variable _PyRuntime?

      What's the long-term goal for _PyRuntime? If you can't get rid of all occurrences of _PyRuntime, how does it help to get rid of *some* occurences?

    7. vstinner commented on Apr 25, 2019

      @vstinner
      MemberAuthor

      I don't really understand the rationale for these changes. What's wrong with the global variable _PyRuntime?
      What's the long-term goal for _PyRuntime? If you can't get rid of all occurrences of _PyRuntime, how does it help to get rid of *some* occurences?

      The long term goal is to support multiple interpreter instances per process:

      Eric Snow's PEP-554 "Multiple Interpreters in the Stdlib"
      https://www.python.org/dev/peps/pep-0554/

      Right now, there is a single instance of _PyRuntimeState: _PyRuntime. I would be interested to have one _PyRuntimeState per interpreter.

      Maybe _PyRuntimeState should be reworked in the meanwhile. Right now, it's a strange beast: it contains things which are set before Python initialization and things which are set after. It contains C types and Python objects. Maybe some parts should be moved into PyInterpreterState or even PyThreadState. I don't know at this point. It takes time to look at each individual structure field...

      Anyway, more generally, IMHO it's a bad practice to rely on a global variable. Python runtime should be "stateless".

      The current implementation of CPython leaks dozens of *Python* objects at exit. For example, I started to work on this issue while working on https://bugzilla.redhat.com/show_bug.cgi?id=1696322 : Python doesn't clear 2 warnings variables at exit. When I looked into this issue, I also noticed that _PyRuntime.gc.garbage remains *alive* after Py_Finalize().

      That's plain wrong: *all* Python objects must be cleared by Py_Finalize(). Two interpreters must *not* share any Python object.

      Well, the PEP should better explain the rationale than me :-)

      --

      When I wrote PR 12934, I noticed that even getting the current thread state rely on _PyRuntime:

      #define _PyThreadState_GET() \
          ((PyThreadState*)_Py_atomic_load_relaxed(&_PyRuntime.gilstate.tstate_current))

      That's wrong in the case of ceval.c: it should be possible to run _PyEval_EvalFrameDefault() twice at the same time in two threads using two "isolated" interpreters.

      Well, PR 12934 doesn't fix all issues. It's just one small step towards "stateless" runtime and the even more long term of having one GIL per *interpeter*, rather than a single GIL per *process*.

    8. vstinner commented on Apr 25, 2019

      @vstinner
      MemberAuthor

      I created bpo-36724: Clear _PyRuntime at exit.

    9. vstinner commented on Apr 25, 2019

      @vstinner
      MemberAuthor

      New changeset 10c8e6a by Victor Stinner in branch 'master':
      bpo-36710: Add runtime variable in pystate.c (GH-12956)
      10c8e6a

    10. vstinner commented on Apr 26, 2019

      @vstinner
      MemberAuthor

      New changeset 9db0324 by Victor Stinner in branch 'master':
      bpo-36710: Add runtime parameter in gcmodule.c (GH-12958)
      9db0324

    11. vstinner commented on Apr 26, 2019

      @vstinner
      MemberAuthor

      New changeset 99e69d4 by Victor Stinner in branch 'master':
      bpo-36710: Fix compiler warning on PyThreadState_Delete() (GH-12962)
      99e69d4

    12. jdemeyer commented on Apr 26, 2019

      @jdemeyer
      Contributor

      The long term goal is to support multiple interpreter instances per process:
      Eric Snow's PEP-554 "Multiple Interpreters in the Stdlib"
      https://www.python.org/dev/peps/pep-0554/

      Sorry, but I don't see the relation between this issue and PEP-554. It seems to me that the PEP is about making subinterpreters available from pure Python (instead of only at the C level). It doesn't say anything about the *implementation* of subinterpreters, which is what this issue is about.

      So I'm still missing the bigger picture where this issue fits in.

      The current implementation of CPython leaks dozens of *Python* objects at exit.

      That may be an issue to be fixed, but again I don't see the relation with this issue.

    13. vstinner commented on Apr 26, 2019

      @vstinner
      MemberAuthor

      Jeroen Demeyer:

      Sorry, but I don't see the relation between this issue and PEP-554.

      The long term plan for PEP-554 is to support having one GIL per interpreter for best performances. The GIL lives in _PyRuntime.

      It's not just about the GIL. Currently, the gc module stores its state into _PyRuntime. It's wrong to share a single gc state between two interpreters: each interpreter should have its own "namespace" completely isolated from the other namespaces. For example, _PyRuntime.gc.garbage is a Python list: each interpreter should have its own list.

      My PR 12934 is only a first step to prepare ceval.c for that.

      Said differently, if I understood correctly, each interpreter must have its own _PyRuntime instance.

      Maybe tomorrow, we will keep a single _PyRuntime instance, *but* my work is needed to identify the relationship between the current implementation of Python and _PyRuntime.

    14. 14 remaining items

    15. vstinner commented on Jun 19, 2019

      @vstinner
      MemberAuthor

      New changeset 0a28f8d by Victor Stinner in branch 'master':
      bpo-36710: Add tstate parameter in import.c (GH-14218)
      0a28f8d

    16. vstinner commented on Jun 19, 2019

      @vstinner
      MemberAuthor

      New changeset 987a0dc by Victor Stinner in branch 'master':
      bpo-36710: Remove PyImport_Cleanup() function (GH-14221)
      987a0dc

    17. vstinner commented on Jun 19, 2019

      @vstinner
      MemberAuthor

      New changeset b45d259 by Victor Stinner in branch 'master':
      bpo-36710: Use tstate in pylifecycle.c (GH-14249)
      b45d259

    18. vstinner commented on Nov 12, 2019

      @vstinner
      MemberAuthor

      I started a thread on python-dev about this issue:
      "Pass the Python thread state to internal C functions"
      https://mail.python.org/archives/list/python-dev@python.org/thread/PQBGECVGVYFTVDLBYURLCXA3T7IPEHHO/

    19. vstinner commented on Nov 20, 2019

      @vstinner
      MemberAuthor

      New changeset 2e96906 by Victor Stinner in branch 'master':
      bpo-36710: Pass tstate parameter to GC collect() (GH-17267)
      2e96906

    20. vstinner commented on Nov 20, 2019

      @vstinner
      MemberAuthor

      New changeset 01b1cc1 by Victor Stinner in branch 'master':
      bpo-36710: Add PyInterpreterState.runtime field (GH-17270)
      01b1cc1

    21. vstinner commented on Jan 13, 2020

      @vstinner
      MemberAuthor

      I continued this work by passing tstate to internal C functions: bpo-38644.

      I also added PyInterpreterState.runtime field, so it's now possible to retrieve the runtime from tstate:

          runtime = tstate->interp->runtime;

      I wrote an article on passing tstate to internal C functions:

      https://vstinner.github.io/cpython-pass-tstate.html

      I consider that this issue is now done. I close the issue.

    22. ericsnowcurrently commented on Jan 17, 2020

      @ericsnowcurrently
      Member

      Thanks, Victor!

    23. markshannon commented on Mar 17, 2020

      @markshannon
      Member

      Instead of passing _PyRuntimeState around everywhere, why not just let it disappear in time.

      Currently _PyRuntimeState manages "global" state, mainly the GIL and some config.
      Once the GIL has been migrated to the sub-interpreters, the config part can be factored out and _PyRuntimeState can just disappear.

    24. zooba commented on Mar 17, 2020

      @zooba
      Member

      Instead of passing _PyRuntimeState around everywhere, why not just let it disappear in time.

      Agreed. It's valuable to pass the thread state, but the runtime state should only be needed to create a new thread state (and arguably not even then).

    25. vstinner commented on Mar 17, 2020

      @vstinner
      MemberAuthor

      Instead of passing _PyRuntimeState around everywhere, why not just let it disappear in time.

      Passing runtime (_PyRuntimeState) is a temporary move until more and more fields are moved from _PyRuntimeState into PyInterpreterState. I just created bpo-39984 "Move some ceval fields from _PyRuntime.ceval to PyInterpreterState.ceval" yesterday ;-)

      Once we will manage to move the GIL into PyInterpreterState, we would still have to pass PyInterpreterState or PyThreadState to function which require to access the GIL. Passing explicitly runtime is a first step to prepare to migration to PyInterpreterState or PyThreadState. My intent is to show that many functions rely on "global variables": pass these variables instead.

      If you are thinking about getting the current Python thread state using a thread local storage, that's a different topic and I'm not aware of an open issue to track this idea.

      Currently _PyRuntimeState manages "global" state, mainly the GIL and some config. Once the GIL has been migrated to the sub-interpreters, the config part can be factored out and _PyRuntimeState can just disappear.

      I don't think that we will be able to fully remove _PyRuntimeState. It seems like the PEP-554 "Multiple Interpreters in the Stdlib" requires a registry of interpreters and it currently lives in _PyRuntimeState.

    26. markshannon commented on Mar 17, 2020

      @markshannon
      Member

      Even if _PyRuntime ends up as just a list of interpreters and doesn't disappear completely, it won't be used anything like as much as it is now.

      Many of the functions that it getting passed to will no longer need it, so why bother passing it now?

    27. vstinner commented on Jun 23, 2020

      @vstinner
      MemberAuthor

      New changeset 61b6492 by Victor Stinner in branch 'master':
      bpo-36710: Pass tstate explicitly in abstract.c (GH-21075)
      61b6492

    28. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.8 (EOL)end of lifeinterpreter-core(Objects, Python, Grammar, and Parser dirs)

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions