classification
Title: sys.setrecursionlimit() must fail if the current recursion depth is higher than the new low-water mark
Type: crash Stage:
Components: Interpreter Core, Tests Versions: Python 3.6, Python 3.4, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: emptysquare, pitrou, python-dev, r.david.murray, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2015-09-29 23:45 by vstinner, last changed 2015-12-09 21:57 by emptysquare. This issue is now closed.

Files
File name Uploaded Description Edit
sys_setrecursionlimit.patch vstinner, 2015-10-01 07:41 review
sys_setrecursionlimit-2.patch vstinner, 2015-10-01 09:57 review
sys_setrecursionlimit-3.patch vstinner, 2015-10-01 10:55 review
end_rec_check.patch vstinner, 2015-10-01 11:06 review
sys_setrecursionlimit-4.patch vstinner, 2015-10-01 21:50 review
Messages (18)
msg251897 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-09-29 23:45
haypo@selma$ ./python -m test -R 3:3: test_sys 
[1/1] test_sys
Fatal Python error: Cannot recover from stack overflow.

Current thread 0x00007f1921854700 (most recent call first):
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/unittest/case.py", line 176 in handle
  File "/home/haypo/prog/python/default/Lib/unittest/case.py", line 727 in assertRaises
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 215 in test_recursionlimit_recovery
  File "/home/haypo/prog/python/default/Lib/unittest/case.py", line 600 in run
  File "/home/haypo/prog/python/default/Lib/unittest/case.py", line 648 in __call__
  File "/home/haypo/prog/python/default/Lib/unittest/suite.py", line 122 in run
  File "/home/haypo/prog/python/default/Lib/unittest/suite.py", line 84 in __call__
  File "/home/haypo/prog/python/default/Lib/unittest/suite.py", line 122 in run
  File "/home/haypo/prog/python/default/Lib/unittest/suite.py", line 84 in __call__
  File "/home/haypo/prog/python/default/Lib/test/support/__init__.py", line 1674 in run
  File "/home/haypo/prog/python/default/Lib/test/support/__init__.py", line 1775 in _run_suite
  File "/home/haypo/prog/python/default/Lib/test/support/__init__.py", line 1809 in run_unittest
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 1098 in test_main
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/runtest.py", line 165 in runtest_inner
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/runtest.py", line 129 in runtest
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/runtest.py", line 60 in runtest_ns
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/main.py", line 271 in run_test
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/main.py", line 293 in run_tests_sequential
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/main.py", line 336 in run_tests
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/main.py", line 367 in main
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/main.py", line 405 in main
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/main.py", line 427 in main_in_temp_cwd
  File "/home/haypo/prog/python/default/Lib/test/__main__.py", line 3 in <module>
  File "/home/haypo/prog/python/default/Lib/runpy.py", line 85 in _run_code
  File "/home/haypo/prog/python/default/Lib/runpy.py", line 170 in _run_module_as_main
Fatal Python error: Aborted

Current thread 0x00007f1921854700 (most recent call first):
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 209 in f
  File "/home/haypo/prog/python/default/Lib/unittest/case.py", line 176 in handle
  File "/home/haypo/prog/python/default/Lib/unittest/case.py", line 727 in assertRaises
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 215 in test_recursionlimit_recovery
  File "/home/haypo/prog/python/default/Lib/unittest/case.py", line 600 in run
  File "/home/haypo/prog/python/default/Lib/unittest/case.py", line 648 in __call__
  File "/home/haypo/prog/python/default/Lib/unittest/suite.py", line 122 in run
  File "/home/haypo/prog/python/default/Lib/unittest/suite.py", line 84 in __call__
  File "/home/haypo/prog/python/default/Lib/unittest/suite.py", line 122 in run
  File "/home/haypo/prog/python/default/Lib/unittest/suite.py", line 84 in __call__
  File "/home/haypo/prog/python/default/Lib/test/support/__init__.py", line 1674 in run
  File "/home/haypo/prog/python/default/Lib/test/support/__init__.py", line 1775 in _run_suite
  File "/home/haypo/prog/python/default/Lib/test/support/__init__.py", line 1809 in run_unittest
  File "/home/haypo/prog/python/default/Lib/test/test_sys.py", line 1098 in test_main
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/runtest.py", line 165 in runtest_inner
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/runtest.py", line 129 in runtest
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/runtest.py", line 60 in runtest_ns
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/main.py", line 271 in run_test
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/main.py", line 293 in run_tests_sequential
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/main.py", line 336 in run_tests
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/main.py", line 367 in main
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/main.py", line 405 in main
  File "/home/haypo/prog/python/default/Lib/test/libregrtest/main.py", line 427 in main_in_temp_cwd
  File "/home/haypo/prog/python/default/Lib/test/__main__.py", line 3 in <module>
  File "/home/haypo/prog/python/default/Lib/runpy.py", line 85 in _run_code
  File "/home/haypo/prog/python/default/Lib/runpy.py", line 170 in _run_module_as_main
Abandon (core dumped)
msg251898 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-09-29 23:57
Oh, it's even worse: "./python -m test test_sys" is enough to reproduce the crash. It looks like the crashs was introduced by the following change which (indirectly) adds one more Python frame in the code to execute test_sys.

changeset:   98417:281ab7954d7c
user:        Victor Stinner <victor.stinner@gmail.com>
date:        Tue Sep 29 23:36:27 2015 +0200
files:       Lib/test/libregrtest/main.py
description:
Issue #25220: Enhance regrtest --coverage

Add a new Regrtest.run_test() method to ensure that --coverage pass the same
options to the runtest() function.
msg251899 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-09-30 00:05
New changeset 00c1cd1f0131 by Victor Stinner in branch 'default':
Issue #25274: Workaround test_sys crash just to keep buildbots usable
https://hg.python.org/cpython/rev/00c1cd1f0131
msg251974 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-09-30 22:42
At revision 281ab7954d7c, the test_recursionlimit_recovery() test of test_sys is executed at depth 36.

The overflowed flag is reset when the depth becomes smaller than 50 * 3 // 4 = 37, so when Py_LeaveRecursiveCall() is called with depth level 36.

The test raises a RecursionError once which sets the overflowed flag. Then it tries again to raise the RecursionError, but instead Py_FatalError() is called because the overflowed flag was not reset.
msg251992 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-01 04:35
See also issue25222. _Py_CheckRecursiveCall may be being called recursively through Py_FatalError.
msg252001 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-10-01 06:57
New changeset 60c4fd84ef92 by Victor Stinner in branch '3.4':
Issue #25274: test_recursionlimit_recovery() of test_sys now checks
https://hg.python.org/cpython/rev/60c4fd84ef92

New changeset 898a9a959927 by Victor Stinner in branch '3.5':
(Merge 3.4) Issue #25274: test_recursionlimit_recovery() of test_sys now checks
https://hg.python.org/cpython/rev/898a9a959927

New changeset bae0912dd160 by Victor Stinner in branch 'default':
(Merge 3.5) Issue #25274: test_recursionlimit_recovery() of test_sys now checks
https://hg.python.org/cpython/rev/bae0912dd160
msg252004 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-10-01 07:41
Attached patch raises a ValueError if the current recursion depth is higher than the new "low-water mark". The low-water mark is the value computed by _Py_MakeEndRecCheck() in Py_LeaveRecursiveCall() to decide if we can reset the overflowed field of the thread state to 0.

Example of error with the patch:

$ ./python -c 'import sys; sys.setrecursionlimit(2)'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ValueError: cannot set recursion limit to 2: the minimum recursion limit is 1 at the recursion depth 1
msg252010 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-01 09:19
> ValueError: cannot set recursion limit to 2: the minimum recursion limit is 1 at the recursion depth 1

This message looks confusing to me. 2 > 1, isn't?

The dependency of min_limit from new_limit is not monotonic:

new_limit = 99 => min_limit = 74
new_limit = 100 => min_limit = 75
new_limit = 101 => min_limit = 51
msg252014 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-10-01 09:57
sys_setrecursionlimit-2.patch: fix error message, exchange the last 2 PyErr_Format() parameters.
msg252017 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-10-01 10:55
Serhiy wrote:
> This message looks confusing to me. 2 > 1, isn't?
> The dependency of min_limit from new_limit is not monotonic: (...)

Ok, now I'm confused too :-)

First of all, I propose to change the exception type to RecursionError because it's really strange to get a ValueError depending on the current recursion depth.

I would prefer to have an hardcoded minimum limit instead of a minimum depending on the current recursion depth, but I'm not sure that it's technically possible according to current constraints in CPython.

Updated patch (version 3) doesn't mention the computed "minimum limit" in the error message since it's hard to compute it for the user (and even for me :-)). For the user, it's hard to estimate (or compute) the current recursion depth.

Updated example:

$ ./python -c 'import sys; sys.setrecursionlimit(0)'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ValueError: recursion limit must be greater or equal than 1

$ ./python -c 'import sys; sys.setrecursionlimit(1)'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
RecursionError: cannot set recursion limit to 1 at the recursion depth 1: the limit is too low

$ ./python -c 'import sys; sys.setrecursionlimit(2)'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
RecursionError: cannot set recursion limit to 2 at the recursion depth 1: the limit is too low

$ ./python -c 'import sys; sys.setrecursionlimit(3)'

(A limit of 3 is valid.)
msg252018 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-10-01 11:06
> The dependency of min_limit from new_limit is not monotonic: (...)

Right, _Py_MakeEndRecCheck() is not monotonic.

Let me try to make it monotonic:

def f1(x): return x * 3 // 4
def f2(x): return x - 50

f1() > f2() for x <= 196
f1() == f2() for x in 198..200
f1() < f2() for x > 201

So I propose to switch between f1() and f2() at x>200 for _Py_MakeEndRecCheck(). It gives:

recursion depth => low-water mark

25 => 18
...
50 => 37
...
75 => 56
...
100 => 75
...
125 => 93
...
150 => 112
...
175 => 131
...
198 => 148 -- use f1 (x*3/4)
199 => 149
200 => 150 
201 => 151 -- switch to f2 (x-50)
202 => 152
203 => 153

Attached end_rec_check.patch makes _Py_MakeEndRecCheck() monotonic.
msg252057 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-01 20:27
_Py_MakeEndRecCheck() was changed in issue5392 (noised Antoine as a committer). There are many ways to make it monotonic.

#define _Py_MakeEndRecCheck(x) \
	(--(x) < ((_Py_CheckRecursionLimit > 200) \
		? (_Py_CheckRecursionLimit - 25) \
		: (3 * (_Py_CheckRecursionLimit >> 2))))

#define _Py_MakeEndRecCheck(x) \
	(--(x) < ((_Py_CheckRecursionLimit > 100) \
		? (_Py_CheckRecursionLimit - 50) \
		: (_Py_CheckRecursionLimit >> 1)))

Since the formula is so complex, it would be worth to extract common part from _Py_MakeEndRecCheck() and sys_setrecursionlimit(). Otherwise the code can become desynchronized.

The error message still looks confusing to me. May be short it to "limit is too low"? Noised David as grammar expert.

Why not have an hardcoded minimum limit? Current recursion depth and therefore minimal recursion limit are implementation defined and can depend on a way how the module is executed (run a script, import a module, run with runpy or IDLE, etc). May be interpret the limit relatively to current recursion depth?
msg252061 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-10-01 21:50
sys_setrecursionlimit-4.patch:

- enhance documentation and comments
- combined with end_rec_check.patch: make the lower-water mark monotonic
- add _Py_RecursionLimitLowerWaterMark() macro used by _Py_MakeEndRecCheck() and sys.setrecursionlimit()
msg252064 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-10-01 21:59
> _Py_MakeEndRecCheck() was changed in issue5392 (noised Antoine as a committer).

Oh, I didn't know this issue. If I am right, it's a duplicate of this issue, even if it was a little bit different because the lower-water mark was computed differently.

> There are many ways to make it monotonic.

Since I don't understand well this issue, I prefer to keep the formula unchanged for low limit (smaller than 100, or smaller than 200 with my patch) to keep the 3/4 ratio.

> Since the formula is so complex, it would be worth to extract common part from _Py_MakeEndRecCheck() and sys_setrecursionlimit(). Otherwise the code can become desynchronized.

Right, it's now done in my new patch.

> The error message still looks confusing to me. May be short it to "limit is too low"? Noised David as grammar expert.

I prefer to really explain the fact that the RecursionError is raised depending on the current recursion depth. Otherwise, I'm quite sure that someone will complain that "too low" have different values on the same PC but on two different applications.

> Why not have an hardcoded minimum limit? Current recursion depth and therefore minimal recursion limit are implementation defined and can depend on a way how the module is executed (run a script, import a module, run with runpy or IDLE, etc).

I'm not sure that you understood correctly the issue. The root cause is that you cannot call sys.setrecursionlimit(X) at recursion depth Y. There is no constant "minimum limit": it depends on the recusion depth because of how Python handles recursion error (the overflowed flag).

An alternative would be to remove completly the overflowed flag with its fatal error. It was introduced during large refactoring of Python, maybe the bug cannot occur anymore?

Again, since I don't know the whole history of how Python handles recursion error, I prefer the conservative approach of changing the minimum amount of code.

> May be interpret the limit relatively to current recursion depth?

Hum, I dislike this idea. I prefer to keep an absolute value. Otherwise, it would make Python more surprising.
msg252065 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-10-01 22:03
> An alternative would be to remove completly the overflowed flag with its fatal error. It was introduced during large refactoring of Python, maybe the bug cannot occur anymore?

By the way, it doesn't exist in Python 2 at all. Try attached double_recursion_error.py program.


$ python2 double_recursion_error.py 
first recursion error
second recursion error


$ python3 double_recursion_error.py 
first recursion error
Fatal Python error: Cannot recover from stack overflow.

Current thread 0x00007f80a6985700 (most recent call first):
  File "double_recursion_error.py", line 5 in f
  File "double_recursion_error.py", line 5 in f
  File "double_recursion_error.py", line 5 in f
  File "double_recursion_error.py", line 5 in f
  ...
Abandon (core dumped)
msg252066 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-10-01 22:07
The overflowed flag was introduced 8 years ago (near the release of Python 3.0) by the following changeset:

changeset:   42013:cd125fe83051
user:        Martin v. Löwis <martin@v.loewis.de>
date:        Sun Jun 10 09:51:05 2007 +0000
files:       Include/ceval.h Include/pystate.h Include/stringobject.h Include/unicodeobject.h Lib/test/test_frozen.py Lib/test/test_new.py Lib/test/test
description:
Make identifiers str (not str8) objects throughout.
This affects the parser, various object implementations,
and all places that put identifiers into C string literals.

In testing, a number of crashes occurred as code would
fail when the recursion limit was reached (such as the
Unicode interning dictionary having key/value pairs where
key is not value). To solve these, I added an overflowed
flag, which allows for 50 more recursions after the
limit was reached and the exception was raised, and
a recursion_critical flag, which indicates that recursion
absolutely must be allowed, i.e. that a certain call
must not cause a stack overflow exception.

There are still some places where both str and str8 are
accepted as identifiers; these should eventually be
removed.
msg252894 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-10-12 22:16
New changeset eb0c76442cee by Victor Stinner in branch '3.5':
sys.setrecursionlimit() now raises RecursionError
https://hg.python.org/cpython/rev/eb0c76442cee
msg252895 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-10-12 22:19
I pushed sys_setrecursionlimit-4.patch.

Thanks a lot Serhiy for your review and your help on this issue!
History
Date User Action Args
2015-12-09 21:57:36emptysquaresetnosy: + emptysquare
2015-10-12 22:19:15vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg252895
2015-10-12 22:16:33python-devsetmessages: + msg252894
2015-10-01 22:07:10vstinnersetmessages: + msg252066
2015-10-01 22:03:54vstinnersetmessages: + msg252065
2015-10-01 21:59:07vstinnersetmessages: + msg252064
2015-10-01 21:50:16vstinnersetfiles: + sys_setrecursionlimit-4.patch

messages: + msg252061
2015-10-01 20:27:16serhiy.storchakasetnosy: + pitrou, r.david.murray
messages: + msg252057
2015-10-01 11:06:35vstinnersetfiles: + end_rec_check.patch

messages: + msg252018
2015-10-01 10:55:04vstinnersetfiles: + sys_setrecursionlimit-3.patch

messages: + msg252017
2015-10-01 09:57:11vstinnersetfiles: + sys_setrecursionlimit-2.patch

messages: + msg252014
2015-10-01 09:19:31serhiy.storchakasetmessages: + msg252010
2015-10-01 07:42:19vstinnersetversions: + Python 3.4, Python 3.5
2015-10-01 07:41:09vstinnersetfiles: + sys_setrecursionlimit.patch
keywords: + patch
messages: + msg252004

title: "./python -m test test_sys": Fatal Python error: Cannot recover from stack overflow -> sys.setrecursionlimit() must fail if the current recursion depth is higher than the new low-water mark
2015-10-01 06:57:37python-devsetmessages: + msg252001
2015-10-01 04:35:28serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg251992

components: + Interpreter Core, Tests
type: crash
2015-09-30 22:42:27vstinnersetmessages: + msg251974
2015-09-30 00:05:13python-devsetnosy: + python-dev
messages: + msg251899
2015-09-29 23:57:30vstinnersetmessages: + msg251898
title: "./python -m test -R 3:3: test_sys": Fatal Python error: Cannot recover from stack overflow -> "./python -m test test_sys": Fatal Python error: Cannot recover from stack overflow
2015-09-29 23:45:48vstinnercreate