The Wayback Machine - https://web.archive.org/web/20210910033958/https://github.com/python/cpython/pull/28190
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

bpo-44348: BaseException deallocator uses trashcan #28190

Merged
merged 2 commits into from Sep 7, 2021

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Sep 6, 2021

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

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
Copy link
Member Author

@vstinner vstinner commented Sep 6, 2021

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
Copy link
Member

@pablogsal pablogsal commented Sep 6, 2021

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

@vstinner
Copy link
Member Author

@vstinner vstinner commented Sep 6, 2021

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.

@corona10 corona10 self-requested a review Sep 7, 2021
Copy link
Member

@corona10 corona10 left a comment

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

@corona10 corona10 requested a review from pablogsal Sep 7, 2021
@vstinner vstinner merged commit fb30509 into python:main Sep 7, 2021
12 checks passed
12 checks passed
@github-actions[bot]
Docs Docs
Details
@github-actions[bot]
Check for source changes
Details
@github-actions[bot]
Check if generated files are up to date
Details
@github-actions[bot]
Windows (x86)
Details
@github-actions[bot]
Windows (x64)
Details
@github-actions[bot]
macOS
Details
@github-actions[bot]
Ubuntu
Details
@github-actions[bot]
Ubuntu SSL tests with OpenSSL
Details
@github-actions[bot]
Address sanitizer Address sanitizer
Details
Azure Pipelines PR #20210906.22 succeeded
Details
@bedevere-bot
bedevere/issue-number Issue number 44348 found
Details
@bedevere-bot
bedevere/news News entry found in Misc/NEWS.d
@vstinner vstinner deleted the exc_trashcan branch Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants