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

Remove _PyThreadState_GetFrame #84127

Closed
vstinner opened this issue Mar 12, 2020 · 7 comments
Closed

Remove _PyThreadState_GetFrame #84127

vstinner opened this issue Mar 12, 2020 · 7 comments
Labels
3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@vstinner
Copy link
Member

BPO 39946
Nosy @brettcannon, @gpshead, @vstinner, @markshannon
PRs
  • bpo-39946: Remove _PyThreadState_GetFrame #19094
  • 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-03-20.16:49:31.422>
    created_at = <Date 2020-03-12.17:40:48.726>
    labels = ['interpreter-core', '3.9']
    title = 'Remove _PyThreadState_GetFrame'
    updated_at = <Date 2020-05-04.21:50:53.284>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2020-05-04.21:50:53.284>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-03-20.16:49:31.422>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2020-03-12.17:40:48.726>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39946
    keywords = ['patch']
    message_count = 7.0
    messages = ['364031', '364675', '364677', '364679', '364680', '368084', '368085']
    nosy_count = 4.0
    nosy_names = ['brett.cannon', 'gregory.p.smith', 'vstinner', 'Mark.Shannon']
    pr_nums = ['19094']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue39946'
    versions = ['Python 3.9']

    @vstinner
    Copy link
    Member Author

    Python has an internal function to get the frame of the PyThreadState:

    /* hook for PyEval_GetFrame(), requested for Psyco */
    #define _PyThreadState_GetFrame _PyRuntime.gilstate.getframe

    It is used by the public function PyEval_GetFrame() for example.

    The indirection was added in 2002 by:

    commit 019a78e
    Author: Michael W. Hudson <mwh@python.net>
    Date: Fri Nov 8 12:53:11 2002 +0000

    Assorted patches from Armin Rigo:
    
    [ 617309 ] getframe hook (Psyco #1)
    [ 617311 ] Tiny profiling info (Psyco #2)
    [ 617312 ] debugger-controlled jumps (Psyco #3)
    
    These are forward ports from 2.2.2.
    

    ... but psyco is outdated for a very long time (superseded by PyPy which is no longer based on CPython). Is it time to drop _PyThreadState_GetFrame() (which became _PyRuntime.gilstate.getframe in the meanwhile)?

    Or if we keep it, we should use it rather accessing directly PyThreadState.frame (read or write).

    See also PEP-523 "Adding a frame evaluation API to CPython" and a recent discussion on this PEP: bpo-38500.

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

    I added PyThreadState_GetFrame() function in bpo-39947: commit fd1e1a1.

    @vstinner
    Copy link
    Member Author

    It looks safe to remove this feature.

    I failed to find any recent user of _PyThreadState_GetFrame.

    Moreover, _PyRuntime.getframe and _PyThreadState_GetFrame have been moved to the internal C API in Python 3.7. This C API was not accessible by third-party projects in Python 3.7. The internal C API can only be used by third-party projects since Python 3.8 and it's tricky to use it... on purpose. I don't recall any complain about _PyThreadState_GetFrame becoming inaccessible.

    --

    I searched for "_PyThreadState_GetFrame" in C code on GitHub. I only found copies of Python/pystate.c files in the first 10 pages of search results, with one exception. I found one file:

    https://github.com/Mistobaan/pdb-clone/blob/517f6d19902b64395b4c7218cbbbecfa5a1de607/lib/pdb_clone/_pdbhandler-py27.c

    It's an old (latest commit on Mar 31, 2015) debugger project written by Xavier de Gaye.

    It seems like the following flavor is more recent (latest commit on Apr 20, 2019):

    https://github.com/corpusops/pdbclone/

    Note: the project contains 4 .c files, but only _pdbhandler-py27.c uses _PyThreadState_GetFrame: _pdbhandler-py3.c which is for Python 3 doesn't use _PyThreadState_GetFrame. But Python 2.7 reached its end of life, it's no longer supported.

    Copy of the code:

    /* Disable the Python 2 restricted mode in the subinterpreter (see
     * PyEval_GetRestricted()) that prevents linecache to open the source
     * files and prevents attribute access. */
    saved_globals = mainstate->frame->f_globals;
    saved_locals = mainstate->frame->f_locals;
    saved_tstate_getframe = _PyThreadState_GetFrame;
    mainstate->frame->f_globals = globals;
    mainstate->frame->f_locals = locals;
    _PyThreadState_GetFrame = threadstate_getframe;
    pdbhandler_tstate = mainstate;
    

    So _PyThreadState_GetFrame was used to "disable the Python 2 restricted mode", but this mode has been removed from Python 3.

    @vstinner vstinner changed the title Is it time to remove _PyThreadState_GetFrame() hook? Remove _PyThreadState_GetFrame Mar 20, 2020
    @vstinner vstinner changed the title Is it time to remove _PyThreadState_GetFrame() hook? Remove _PyThreadState_GetFrame Mar 20, 2020
    @vstinner
    Copy link
    Member Author

    New changeset 6723e93 by Victor Stinner in branch 'master':
    bpo-39946: Remove _PyThreadState_GetFrame (GH-19094)
    6723e93

    @vstinner
    Copy link
    Member Author

    I removed the function.

    FYI I asked Armin Rigo, the author of _PyThreadState_GetFrame, if PyPy uses it: no, it doesn't. Moreover, psyco project is outdated, only supports Python 2.6 and older, and is superseded by PyPy.

    @gpshead
    Copy link
    Member

    gpshead commented May 4, 2020

    Thanks!

    We use this function internally in some VM traceback grabbing code but the best solution looks to just be for us to adopt the patch to 3.9 on our interpreter until we're running on 3.9.

    @vstinner
    Copy link
    Member Author

    vstinner commented May 4, 2020

    We use this function internally in some VM traceback grabbing code

    Would you mind to elaborate your use case?

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants