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

Add private _PyThreadState_UncheckedGet() to get the current thread state #70342

Closed
vstinner opened this issue Jan 19, 2016 · 6 comments
Closed

Comments

@vstinner
Copy link
Member

BPO 26154
Nosy @gpshead, @vstinner, @larryhastings, @njsmith
Files
  • pythreadstate_fastget.patch
  • 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 2016-01-20.10:21:22.684>
    created_at = <Date 2016-01-19.12:30:33.161>
    labels = []
    title = 'Add private _PyThreadState_UncheckedGet() to get the current thread state'
    updated_at = <Date 2016-04-25.15:16:37.222>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2016-04-25.15:16:37.222>
    actor = 'wenzel'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-01-20.10:21:22.684>
    closer = 'vstinner'
    components = []
    creation = <Date 2016-01-19.12:30:33.161>
    creator = 'vstinner'
    dependencies = []
    files = ['41660']
    hgrepos = []
    issue_num = 26154
    keywords = ['patch']
    message_count = 6.0
    messages = ['258587', '258598', '258633', '258656', '258657', '264178']
    nosy_count = 6.0
    nosy_names = ['gregory.p.smith', 'vstinner', 'larry', 'njs', 'python-dev', 'wenzel']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue26154'
    versions = ['Python 3.5', 'Python 3.6']

    @vstinner
    Copy link
    Member Author

    The issue bpo-25150 modified pystate.h to hide _PyThreadState_Current. Sadly, this change broke the vmprof project:
    https://mail.python.org/pipermail/python-dev/2016-January/142767.html

    Attached patches adds a new private _PyThreadState_FastGet() function to get the current thread state but don't call Py_FatalError() if it is NULL.

    The patch also uses replace direct access to _PyThreadState_Current with _PyThreadState_FastGet(), except inside ceval.c and pystate.c.

    Calling Py_FatalError() to handle errors is not really a great API... Bad that's a different story, I don't want to break anything here.

    I want to add the private function to Python 3.5.2 because I consider that the removal of the _PyThreadState_Current symbol is a regression introduced in Python 3.5.1.

    We have no rule for the Python private API, it can change *anytime*.

    @njsmith
    Copy link
    Contributor

    njsmith commented Jan 19, 2016

    Name should be _PyThreadState_UncheckedGet

    @gpshead
    Copy link
    Member

    gpshead commented Jan 19, 2016

    Overall +1 to this private API. I like the UncheckedGet name better than FastGet but don't really care what the name is so long as it keeps this property: It must be non-blocking and safe to call from a signal handler. Returning NULL in the event the value could not be obtained in such a manner is fine (unlikely to happen from the looks of the 3.5 code).

    They are private and this fixes the regression in 3.5.1. people (ab)using these private APIs should be fine writing conditional compilation code to deal with that.

    We've got a similar patch on our CPython 2.7 interpreter at work (more complicated as it must do a non-blocking lock acquire in the old 2.7 code).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 20, 2016

    New changeset f9461f1e0559 by Victor Stinner in branch '3.5':
    Add _PyThreadState_UncheckedGet()
    https://hg.python.org/cpython/rev/f9461f1e0559

    New changeset d4f13c9a2b07 by Victor Stinner in branch 'default':
    Merge 3.5
    https://hg.python.org/cpython/rev/d4f13c9a2b07

    @vstinner
    Copy link
    Member Author

    Function added

    @fijal: Sorry for the annoyance of the Python 3.5.1 regression.

    @vstinner vstinner changed the title Add private _PyThreadState_FastGet() to get the current thread state Add private _PyThreadState_UncheckedGet() to get the current thread state Jan 20, 2016
    @wenzel
    Copy link
    Mannequin

    wenzel mannequin commented Apr 25, 2016

    I've also run into this regression. FWIW this is what I've ended up using to work around it (it's a mess, but what are we to do..)

    #if PY_VERSION_HEX >= 0x03050000 && PY_VERSION_HEX < 0x03050200
    extern "C" {
        /* Manually import _PyThreadState_Current symbol */
        struct _Py_atomic_address { void *value; };
        PyAPI_DATA(_Py_atomic_address) _PyThreadState_Current;
    };
    #endif
    
    PyThreadState *get_thread_state_unchecked() {
    #if   PY_VERSION_HEX < 0x03000000
        return _PyThreadState_Current;
    #elif PY_VERSION_HEX < 0x03050000
        return (PyThreadState*) _Py_atomic_load_relaxed(&_PyThreadState_Current);
    #elif PY_VERSION_HEX < 0x03050200
        return (PyThreadState*) _PyThreadState_Current.value;
    #else
        return _PyThreadState_UncheckedGet();
    #endif
    }

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    None yet
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants