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

Make the behavior of USE_STACKCHECK deterministic #76038

Closed
pdox mannequin opened this issue Oct 24, 2017 · 3 comments
Closed

Make the behavior of USE_STACKCHECK deterministic #76038

pdox mannequin opened this issue Oct 24, 2017 · 3 comments
Labels
3.7 (EOL) end of life OS-windows type-bug An unexpected behavior, bug, or error

Comments

@pdox
Copy link
Mannequin

pdox mannequin commented Oct 24, 2017

BPO 31857
Nosy @pfmoore, @vstinner, @tjguk, @benjaminp, @zware, @zooba, @pdox
PRs
  • bpo-31857: Make the behavior of USE_STACKCHECK deterministic #4098
  • [WIP] bpo-39984: Add PyInterpreterState.ceval #19034
  • 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 2017-10-26.06:03:32.728>
    created_at = <Date 2017-10-24.01:09:21.961>
    labels = ['type-bug', '3.7', 'OS-windows']
    title = 'Make the behavior of USE_STACKCHECK deterministic'
    updated_at = <Date 2020-03-17.01:04:51.773>
    user = 'https://github.com/pdox'

    bugs.python.org fields:

    activity = <Date 2020-03-17.01:04:51.773>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-10-26.06:03:32.728>
    closer = 'benjamin.peterson'
    components = ['Windows']
    creation = <Date 2017-10-24.01:09:21.961>
    creator = 'pdox'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 31857
    keywords = ['patch']
    message_count = 3.0
    messages = ['304854', '305024', '355965']
    nosy_count = 7.0
    nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'benjamin.peterson', 'zach.ware', 'steve.dower', 'pdox']
    pr_nums = ['4098', '19034']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue31857'
    versions = ['Python 3.7']

    @pdox
    Copy link
    Mannequin Author

    pdox mannequin commented Oct 24, 2017

    USE_STACKCHECK is a Windows-only feature which provides additional safety against C stack overflow by periodically calling PyOS_CheckStack to determine whether the current thread is too close to the end of the stack.

    The way USE_STACKCHECK ensures that PyOS_CheckStack is called frequently is surprising. It does this by artificially decrementing _Py_CheckRecursionLimit with every call to Py_EnterRecursiveCall:

        #ifdef USE_STACKCHECK
        /* With USE_STACKCHECK, we artificially decrement the recursion limit in order
           to trigger regular stack checks in _Py_CheckRecursiveCall(), except if
           the "overflowed" flag is set, in which case we need the true value
           of _Py_CheckRecursionLimit for _Py_MakeEndRecCheck() to function properly.
        */
        #  define _Py_MakeRecCheck(x)  \
            (++(x) > (_Py_CheckRecursionLimit += PyThreadState_GET()->overflowed - 1))
        #else
        #  define _Py_MakeRecCheck(x)  (++(x) > _Py_CheckRecursionLimit)
        #endif

    _Py_CheckRecursionLimit defaults to 1000, and is constant when USE_STACKCHECK is off. (except when changed by Py_SetRecursionLimit)

    With a single thread, each call to Py_EnterRecursiveCall causes _Py_CheckRecursionLimit to decrement by 1 until it equals the current recursion depth, at which point _Py_CheckRecursiveCall is triggered, resetting _Py_CheckRecursionLimit back to the default. This could be anywhere from 500 to 1000 calls depending on the recursion depth. With multiple threads, the behavior may be more chaotic, as each thread will be decrementing _Py_CheckRecursionLimit independently.

    I propose that instead, the call to PyOS_CheckStack is triggered in a predictable fashion, using a separate counter for each thread, so that it occurs every N'th call to Py_EnterRecursiveCall. (N = 64 seems reasonable, as PyOS_CheckStack guarantees 2048 * sizeof(void*) bytes remaining on the C stack).

    @pdox pdox mannequin added 3.7 (EOL) end of life OS-windows type-bug An unexpected behavior, bug, or error labels Oct 24, 2017
    @benjaminp
    Copy link
    Contributor

    New changeset 1896793 by Benjamin Peterson (pdox) in branch 'master':
    bpo-31857: Make the behavior of USE_STACKCHECK deterministic (bpo-4098)
    1896793

    @vstinner
    Copy link
    Member

    vstinner commented Nov 4, 2019

    This issue added this FIXME:

    /* Due to the macros in which it's used, _Py_CheckRecursionLimit is in
    the stable ABI. It should be removed therefrom when possible.
    */

    FYI I proposed PR 17046 to fix it ;-)

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life OS-windows type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants