classification
Title: Make the behavior of USE_STACKCHECK deterministic
Type: behavior Stage: resolved
Components: Windows Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, paul.moore, pdox, steve.dower, tim.golden, vstinner, zach.ware
Priority: normal Keywords: patch

Created on 2017-10-24 01:09 by pdox, last changed 2019-11-04 17:16 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 4098 merged pdox, 2017-10-24 01:31
Messages (3)
msg304854 - (view) Author: (pdox) * Date: 2017-10-24 01:09
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).
msg305024 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2017-10-26 06:03
New changeset 1896793520a49a6f97ae360c0b288967e56b005e by Benjamin Peterson (pdox) in branch 'master':
bpo-31857: Make the behavior of USE_STACKCHECK deterministic (#4098)
https://github.com/python/cpython/commit/1896793520a49a6f97ae360c0b288967e56b005e
msg355965 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-04 17:16
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 ;-)
History
Date User Action Args
2019-11-04 17:16:57vstinnersetnosy: + vstinner
messages: + msg355965
2017-10-26 06:03:32benjamin.petersonsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-10-26 06:03:07benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg305024
2017-10-24 01:31:05pdoxsetkeywords: + patch
stage: patch review
pull_requests: + pull_request4069
2017-10-24 01:09:22pdoxcreate