GeezerVison is showing github.com — Implement stack overflow protection for supported platforms · Issue #91079 · python/cpython
Implement stack overflow protection for supported platforms #91079Closed 2 / 22 of 2 issues completedClosed 2 / 22 of 2 issues completed Description
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 = 'https://github.com/markshannon'
closed_at = None
created_at = <Date 2022-03-04.16:54:19.358>
labels = []
title = 'Implement stack overflow protection for supported platforms'
updated_at = <Date 2022-03-14.14:27:17.389>
user = 'https://github.com/markshannon' bugs.python.org fields:
activity = <Date 2022-03-14.14:27:17.389>
actor = 'ronaldoussoren'
assignee = 'Mark.Shannon'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2022-03-04.16:54:19.358>
creator = 'Mark.Shannon'
dependencies = []
files = []
hgrepos = []
issue_num = 46923
keywords = []
message_count = 3.0
messages = ['414538', '414553', '415143']
nosy_count = 6.0
nosy_names = ['gregory.p.smith', 'ronaldoussoren', 'steven.daprano', 'Mark.Shannon', 'corona10', 'brandtbucher']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue46923'
versions = [] Linked PRs
Activitymarkshannon commented on Mar 4, 2022 python/steering-council#102 (definitely not PEP-651 ;))
We should implement efficient stack checks on those platforms that allow us to introspect stack extents.
Windows and posix systems allow us to do this.
C allows addresses of stack variables to be taken. This means that C stacks cannot be moved.
In theory they might be discontinuous, although I suspect that they are always contiguous.
My plan is to maintain a per-thread "safe region" of 32or 64k. We can check if the current stack pointer (or near enough) is in that region cheaply.
If we are not in that region, we can ask the O/S for a stack limit to determine a new "safe region". If we cannot find a safe region, then we raise a MemoryError.
Personally I'd prefer a new exception StackOverflow to MemoryError but, thanks to stackoverflow.com, it is now impossible for new programmers to do a web search to determine what a "stack overflow" is.
So, I guess MemoryError will have to do. Reacted by Gregory P. Smith stevendaprano commented on Mar 4, 2022
Personally I'd prefer a new exception StackOverflow to MemoryError
+1 on a new exception (presumably a subclass of MemoryError).
How about using OverflowedStack instead?
The situation is not quite as bad as you suggest. Googling for "stack overflow" alone (with a space and no other qualifications):
-
on Bing, scroll halfway down the first page of results to find the "People also ask..."
How do you get a stack overflow?
How to prevent a stack overflow error?
-
also on Bing at the bottom of the first page of results is a question on stackoverflow.com asking what causes memory stack overflows;
-
on DuckDuckGo, the first page of search results fails to suggest anything useful;
-
on Google itself, on the first page is the People Also Ask
What causes stack overflows?
-
as well as a link to Wikipedia's page on stack overflows.
I expect that going forward, "python stack overflow exception" will be sufficient to hit the Python docs somewhere on the first page.
Besides, presumably this OverflowedStack exception is likely to be rare, so I expect that few people will need to google it. ronaldoussoren commented on Mar 14, 2022 bpo-33955 is an older issue about implementing the current functionality for this on macOS, which has an API for querying stack limits. markshannon commented on Aug 12, 2022 It looks like OverflowedStack (or whatever it ends up being called) will have to subclass RecursionError, not MemoryError.
The use of RecursionError, rather than the more general RuntimeError is extensive in the tests, so might well be elsewhere.
But the more serious problem is that MemoryError implies that you have run out of memory, which is not the case with a stack overflow. MemoryError is treated specially in the VM in ways that are not appropriate for a stack overflow.
E.g. no traceback, a fixed number of instances.
So, I think the way to go is to add two subclasses of RecursionError, one for excessively deep Python code, and another for consuming too much C stack.
Another reason for making them both subclasses of RecursionError is that it cannot be determined statically which will occur first.
If the call stack is a mixture of Python and builtin functions, either the Python stack, or the C stack could reach it limits first.
Raising a RecursionError in both cases makes it easier to handle this case.
Given that we are subclassing RecursionError, my preferred names would be CRecursionError and PyRecursionError.
Which brings me on to the other issue.
Portability of stack overflows.
Since we do not use the C stack for Python recursion, and we will hopefully modify the compiler to use less stack, we could reduce the amount of C stack available on linux to more closely match Windows, increasing the portability of code. Code that raises a CRecursionError on one platform, will likely raise the same exception on other platforms. Reacted by Gregory P. Smith gpshead commented on Aug 12, 2022 I'm not sure we should differentiate at the Exception class level between C and Py RecursionError. There is no action someone can take in code catching one vs the other. So just call them all RecursionError and differentiate reasons within the error message string itself in case the user wants some hint as to if there's anything they can tweak to avoid it at the moment. Implementation of APIs can be migrated to/from C or Python and implementations vary how much space gets used over time so anyone only catching one of those instead of the more general exception isn't going to be in a good position for future change compatibility that might alter which one their code sees. gpshead commented on Aug 12, 2022
we could reduce the amount of C stack available on linux to more closely match Windows
I don't recommend this. It would break someones code that is working just fine in their existing environment who has no interest in other environments.
A nicer long term consistent goal would be to allow RecursionError to be eliminated for pure Python recursion entirely by those who want to turn off our default limit. (ultimately they'd hit a MemoryError if their process wasn't OOM killed) markshannon commented on Aug 13, 2022 OK. Let's reuse RecursionError for now, we can also add subclasses later.
There is no need to change the amount C stack either, but I think it is worth considering later.
A nicer long term consistent goal would be to allow RecursionError to be eliminated for pure Python recursion
Eliminating recursion limits is probably a bad idea, we need memory to build all those frame objects and traceback objects.
Try setting the recursion limit to 100_000_000 to see what happens. It takes a long time to build 100M frame objects and 100M traceback objects, and if you hit ctrl-C while they are being created, it just gets worse. Reacted by Gregory P. Smith Reacted by Jackson Holmes-Matheson markshannon commented on Sep 1, 2022
- Implement portable version that doesn't request information from the O/S, so that we have a solid fallback for unsupported platforms
- Allow use of full stack on linux (and other posix) systems
- Allow use of full stack on Windows
- Allow use of full stack on MacOS.
- Better support on webassembly platforms
Reacted by Gregory P. Smith added a commit that references this issue on Oct 5, 2022GH-91079: Decouple C stack overflow checks from Python recursion chec… encukou commented on Feb 24, 2025 I've reverted for now, as buildbot failures can masking other bugs that sneak in to main.
Like any revert, this doesn't mean the feature or the implementation is bad -- just that it needs more work for cases.
Use git cherry-pick 2498c22 to add the change back locally.
If you'd like me to go through this and add #ifdefs everywhere so we can introduce it gradually (on known-good platforms), let me know. markshannon commented on Feb 24, 2025 I really wish you hadn't reverted when I had a mergeable fix: #130398 markshannon commented on Feb 24, 2025 Actually, its not a big deal. I've just rebased that PR on the revert of the revert. brettcannon commented on Feb 24, 2025
I really wish you hadn't reverted when I had a mergeable fix: #130398
Unfortunately, that PR didn't reference this issue so you couldn't tell from the timeline that a PR had been created. Also, the last comment suggested a potential fix on Friday which didn't happen. That left at least the WASI buildbot red for 5 days when PEP 11 policy is 24 hours.
I understand the frustration, but based on the signals in this issue there wasn't a quick fix coming, so unblocking a tier 2 platform took precedence. encukou commented on Jun 9, 2025 It doesn't seem safe to backport this to 3.13, so #113655 should be solved another way and is no longer blocked by this.
Removing deferred-blocker here. brettcannon commented on Jun 9, 2025 @encukou FYI the issue looks to still be in the project board.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|