Skip to content
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

Jump to bottom

Move pending calls from _PyRuntime to PyInterpreterState #84165

Closed
vstinner opened this issue Mar 17, 2020 · 14 comments
Closed

Move pending calls from _PyRuntime to PyInterpreterState #84165

vstinner opened this issue Mar 17, 2020 · 14 comments
Labels
3.9 only security fixes topic-subinterpreters

Comments

@vstinner
Copy link
Member

BPO 39984
Nosy @vstinner, @phsilva, @ericsnowcurrently, @shihai1991
PRs
  • [WIP] bpo-39984: Add PyInterpreterState.ceval #19034
  • bpo-39984: Add PyInterpreterState.ceval #19047
  • bpo-39984: Pass tstate to _PyEval_SignalAsyncExc() #19049
  • bpo-39984: Pass tstate to handle_signals() #19050
  • bpo-39984: _PyThreadState_DeleteCurrent() takes tstate #19051
  • bpo-39877: Fix take_gil() for daemon threads #19054
  • bpo-39984: trip_signal() uses PyGILState_GetThisThreadState() #19061
  • bpo-39984: Move pending calls to PyInterpreterState #19066
    		•

    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-04-08.21:53:48.838>
created_at = <Date 2020-03-17.00:53:21.613>
labels = ['expert-subinterpreters', '3.9']
title = 'Move pending calls from _PyRuntime to PyInterpreterState'
updated_at = <Date 2020-05-15.00:37:52.239>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2020-05-15.00:37:52.239>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2020-04-08.21:53:48.838>
closer = 'vstinner'
components = ['Subinterpreters']
creation = <Date 2020-03-17.00:53:21.613>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 39984
keywords = ['patch']
message_count = 14.0
messages = ['364378', '364467', '364491', '364492', '364493', '364495', '364503', '364549', '364585', '365006', '365161', '365177', '365998', '366018']
nosy_count = 4.0
nosy_names = ['vstinner', 'phsilva', 'eric.snow', 'shihai1991']
pr_nums = ['19034', '19047', '19049', '19050', '19051', '19054', '19061', '19066']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue39984'
versions = ['Python 3.9']
    @vstinner
    Copy link
    Member Author

    The _PyRuntime.ceval structure should be made "per-interpreter". I don't want to make the GIL per-interpreter: that's out of the scope of this issue. So I propose to only move a few fields to make more ceval fields "per interpreter".

    @vstinner vstinner added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.9 only security fixes labels Mar 17, 2020
    @vstinner
    Copy link
    Member Author

    New changeset dab8423 by Victor Stinner in branch 'master':
    bpo-39984: Add PyInterpreterState.ceval (GH-19047)
    dab8423

    @vstinner
    Copy link
    Member Author

    While trying to move pending to PyInterpreterState, I hit in issue in _PyEval_AddPendingCall(): signal.raise_signal() can call it with tstate=NULL.

    See https://bugs.python.org/issue37127#msg364489

    @vstinner
    Copy link
    Member Author

    New changeset d7fabc1 by Victor Stinner in branch 'master':
    bpo-39984: Pass tstate to handle_signals() (GH-19050)
    d7fabc1

    @vstinner
    Copy link
    Member Author

    New changeset 23ef89d by Victor Stinner in branch 'master':
    bpo-39984: _PyThreadState_DeleteCurrent() takes tstate (GH-19051)
    23ef89d

    @vstinner
    Copy link
    Member Author

    New changeset 29356e0 by Victor Stinner in branch 'master':
    bpo-39877: Fix take_gil() for daemon threads (GH-19054)
    29356e0

    @vstinner
    Copy link
    Member Author

    New changeset 56bfdeb by Victor Stinner in branch 'master':
    bpo-39984: Pass tstate to _PyEval_SignalAsyncExc() (GH-19049)
    56bfdeb

    @vstinner
    Copy link
    Member Author

    New changeset 8849e59 by Victor Stinner in branch 'master':
    bpo-39984: trip_signal() uses PyGILState_GetThisThreadState() (GH-19061)
    8849e59

    @vstinner
    Copy link
    Member Author

    New changeset 50e6e99 by Victor Stinner in branch 'master':
    bpo-39984: Move pending calls to PyInterpreterState (GH-19066)
    50e6e99

    @vstinner
    Copy link
    Member Author

    I moved pending calls to PyInterpreterState.

    There are remaining issues with using pending calls in subinterpreters, but I propose to continue the discussion in bpo-37127.

    @vstinner vstinner changed the title Move some ceval fields from _PyRuntime.ceval to PyInterpreterState.ceval Move pending calls from _PyRuntime to PyInterpreterState Mar 25, 2020
    @ericsnowcurrently
    Copy link
    Member

    Awesome! Thanks for doing this, Victor. I'll take a look when I can and adjust the changes for bpo-33608. If you'll recall, I made a similar change as part of the solution for that issue, which we later reverted due to problems we discovered with daemon threads during runtime finalization.

    @vstinner
    Copy link
    Member Author

    Awesome! Thanks for doing this, Victor. I'll take a look when I can and adjust the changes for bpo-33608. If you'll recall, I made a similar change as part of the solution for that issue, which we later reverted due to problems we discovered with daemon threads during runtime finalization.

    I recall the revert dance, yeah. It took one year to fix the daemon threads and related issues! So far, buildbots look happy. I also ran many manual tests which makes me confident that the code is now reliable even with all recent changes done for isolating thread states.

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 8, 2020

    I reopen the issue because of bpo-40082 "trip_signal() gets NULL tstate on Windows on CTRL+C" regression.

    @vstinner vstinner reopened this Apr 8, 2020
    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 8, 2020

    I reopen the issue because of bpo-40082 "trip_signal() gets NULL tstate on Windows on CTRL+C" regression.

    bpo-40082 is fixed, so I close the issue again.

    @vstinner vstinner closed this as completed Apr 8, 2020
    @vstinner vstinner added topic-subinterpreters and removed interpreter-core (Objects, Python, Grammar, and Parser dirs) labels May 15, 2020
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @ericsnowcurrently ericsnowcurrently mentioned this issue May 23, 2023
    Closed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.9 only security fixes topic-subinterpreters
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @vstinner @ericsnowcurrently