bpo-44348: BaseException deallocator uses trashcan by vstinner · Pull Request #28190 · python/cpython (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation4 Commits2 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

vstinner

The deallocator function of the BaseException type now uses the
trashcan mecanism to prevent stack overflow. For example, when a
RecursionError instance is raised, it can be linked to another
RecursionError through the context attribute or the traceback
attribute, and then a chain of exceptions is created. When the chain
is destroyed, nested deallocator function calls can crash with a
stack overflow if the chain is too long compared to the available
stack memory.

https://bugs.python.org/issue44348

@vstinner

The deallocator function of the BaseException type now uses the trashcan mecanism to prevent stack overflow. For example, when a RecursionError instance is raised, it can be linked to another RecursionError through the context attribute or the traceback attribute, and then a chain of exceptions is created. When the chain is destroyed, nested deallocator function calls can crash with a stack overflow if the chain is too long compared to the available stack memory.

@vstinner

cc @pablogsal @corona10 @nascheme

This fix for https://bugs.python.org/issue44348 doesn't try to force inlining Py_TYPE(), it doesn't change compiler options on Windows, it doesn't try to change the stack size. Instead, it uses the more portable trashcan mecanism.

I'm not sure how it is possible that test_exceptions creates a chain of exceptions long enough to crash Python. The test uses a recursion limit around 20 frames. In Visual Studio, when the stack overflow occurs, the call stack is so deep that Visual Studio says that it cannot display it fully (it's truncated).

@pablogsal

The trashcan mechanism is known to have performance impact. Can you run some benchmarks so we can be more comfortable evaluating the solution? :)

@vstinner

The trashcan mechanism is known to have performance impact. Can you run some benchmarks so we can be more comfortable evaluating the solution? :)

I cannot see any overhead on my benchmark which creates up to 10 000 exceptions per iteration: https://bugs.python.org/issue44348#msg401194 Please review the benchamrk.

@vstinner

corona10

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, it's quite amazing that does not give notable overhead.