From 50856d5ae745d1c9f691afbd78572bf073c941cf Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 13 Oct 2015 00:11:21 +0200 Subject: sys.setrecursionlimit() now raises RecursionError Issue #25274: sys.setrecursionlimit() now raises a RecursionError if the new recursion limit is too low depending at the current recursion depth. Modify also the "lower-water mark" formula to make it monotonic. This mark is used to decide when the overflowed flag of the thread state is reset. --- Doc/library/sys.rst | 7 +++++++ Include/ceval.h | 12 ++++++++--- Lib/test/test_sys.py | 52 ++++++++++++++++++++++++++++++++++++++++------- Misc/NEWS | 5 +++++ Modules/_testcapimodule.c | 10 +++++++++ Python/sysmodule.c | 29 +++++++++++++++++++++++--- 6 files changed, 102 insertions(+), 13 deletions(-) diff --git a/Doc/library/sys.rst b/Doc/library/sys.rst index 72f5d1f..f6325cc 100644 --- a/Doc/library/sys.rst +++ b/Doc/library/sys.rst @@ -975,6 +975,13 @@ always available. that supports a higher limit. This should be done with care, because a too-high limit can lead to a crash. + If the new limit is too low at the current recursion depth, a + :exc:`RecursionError` exception is raised. + + .. versionchanged:: 3.5.1 + A :exc:`RecursionError` exception is now raised if the new limit is too + low at the current recursion depth. + .. function:: setswitchinterval(interval) diff --git a/Include/ceval.h b/Include/ceval.h index eb1ee43..b5373a9 100644 --- a/Include/ceval.h +++ b/Include/ceval.h @@ -94,10 +94,16 @@ PyAPI_DATA(int) _Py_CheckRecursionLimit; # define _Py_MakeRecCheck(x) (++(x) > _Py_CheckRecursionLimit) #endif +/* Compute the "lower-water mark" for a recursion limit. When + * Py_LeaveRecursiveCall() is called with a recursion depth below this mark, + * the overflowed flag is reset to 0. */ +#define _Py_RecursionLimitLowerWaterMark(limit) \ + (((limit) > 200) \ + ? ((limit) - 50) \ + : (3 * ((limit) >> 2))) + #define _Py_MakeEndRecCheck(x) \ - (--(x) < ((_Py_CheckRecursionLimit > 100) \ - ? (_Py_CheckRecursionLimit - 50) \ - : (3 * (_Py_CheckRecursionLimit >> 2)))) + (--(x) < _Py_RecursionLimitLowerWaterMark(_Py_CheckRecursionLimit)) #define Py_ALLOW_RECURSION \ do { unsigned char _old = PyThreadState_GET()->recursion_critical;\ diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 8ec38c8..2d95653 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -201,22 +201,60 @@ class SysModuleTest(unittest.TestCase): if hasattr(sys, 'gettrace') and sys.gettrace(): self.skipTest('fatal error if run with a trace function') - # NOTE: this test is slightly fragile in that it depends on the current - # recursion count when executing the test being low enough so as to - # trigger the recursion recovery detection in the _Py_MakeEndRecCheck - # macro (see ceval.h). oldlimit = sys.getrecursionlimit() def f(): f() try: - for i in (50, 1000): - # Issue #5392: stack overflow after hitting recursion limit twice - sys.setrecursionlimit(i) + for depth in (10, 25, 50, 75, 100, 250, 1000): + try: + sys.setrecursionlimit(depth) + except RecursionError: + # Issue #25274: The recursion limit is too low at the + # current recursion depth + continue + + # Issue #5392: test stack overflow after hitting recursion + # limit twice self.assertRaises(RecursionError, f) self.assertRaises(RecursionError, f) finally: sys.setrecursionlimit(oldlimit) + @test.support.cpython_only + def test_setrecursionlimit_recursion_depth(self): + # Issue #25274: Setting a low recursion limit must be blocked if the + # current recursion depth is already higher than the "lower-water + # mark". Otherwise, it may not be possible anymore to + # reset the overflowed flag to 0. + + from _testcapi import get_recursion_depth + + def set_recursion_limit_at_depth(depth, limit): + recursion_depth = get_recursion_depth() + if recursion_depth >= depth: + with self.assertRaises(RecursionError) as cm: + sys.setrecursionlimit(limit) + self.assertRegex(str(cm.exception), + "cannot set the recursion limit to [0-9]+ " + "at the recursion depth [0-9]+: " + "the limit is too low") + else: + set_recursion_limit_at_depth(depth, limit) + + oldlimit = sys.getrecursionlimit() + try: + sys.setrecursionlimit(1000) + + for limit in (10, 25, 50, 75, 100, 150, 200): + # formula extracted from _Py_RecursionLimitLowerWaterMark() + if limit > 200: + depth = limit - 50 + else: + depth = limit * 3 // 4 + set_recursion_limit_at_depth(depth, limit) + finally: + sys.setrecursionlimit(oldlimit) + def test_recursionlimit_fatalerror(self): # A fatal error occurs if a second recursion limit is hit when recovering # from a first one. diff --git a/Misc/NEWS b/Misc/NEWS index 8d69867..783d27d 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -11,6 +11,11 @@ Release date: TBA Core and Builtins ----------------- +- Issue #25274: sys.setrecursionlimit() now raises a RecursionError if the new + recursion limit is too low depending at the current recursion depth. Modify + also the "lower-water mark" formula to make it monotonic. This mark is used + to decide when the overflowed flag of the thread state is reset. + - Issue #24402: Fix input() to prompt to the redirected stdout when sys.stdout.fileno() fails. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index ba0a24b..9a03648 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3518,6 +3518,15 @@ test_PyTime_AsMicroseconds(PyObject *self, PyObject *args) return _PyTime_AsNanosecondsObject(ms); } +static PyObject* +get_recursion_depth(PyObject *self, PyObject *args) +{ + PyThreadState *tstate = PyThreadState_GET(); + + /* substract one to ignore the frame of the get_recursion_depth() call */ + return PyLong_FromLong(tstate->recursion_depth - 1); +} + static PyMethodDef TestMethods[] = { {"raise_exception", raise_exception, METH_VARARGS}, @@ -3694,6 +3703,7 @@ static PyMethodDef TestMethods[] = { #endif {"PyTime_AsMilliseconds", test_PyTime_AsMilliseconds, METH_VARARGS}, {"PyTime_AsMicroseconds", test_PyTime_AsMicroseconds, METH_VARARGS}, + {"get_recursion_depth", get_recursion_depth, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; diff --git a/Python/sysmodule.c b/Python/sysmodule.c index f600baf..334f5d0 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -632,14 +632,37 @@ processor's time-stamp counter." static PyObject * sys_setrecursionlimit(PyObject *self, PyObject *args) { - int new_limit; + int new_limit, mark; + PyThreadState *tstate; + if (!PyArg_ParseTuple(args, "i:setrecursionlimit", &new_limit)) return NULL; - if (new_limit <= 0) { + + if (new_limit < 1) { PyErr_SetString(PyExc_ValueError, - "recursion limit must be positive"); + "recursion limit must be greater or equal than 1"); return NULL; } + + /* Issue #25274: When the recursion depth hits the recursion limit in + _Py_CheckRecursiveCall(), the overflowed flag of the thread state is + set to 1 and a RecursionError is raised. The overflowed flag is reset + to 0 when the recursion depth goes below the low-water mark: see + Py_LeaveRecursiveCall(). + + Reject too low new limit if the current recursion depth is higher than + the new low-water mark. Otherwise it may not be possible anymore to + reset the overflowed flag to 0. */ + mark = _Py_RecursionLimitLowerWaterMark(new_limit); + tstate = PyThreadState_GET(); + if (tstate->recursion_depth >= mark) { + PyErr_Format(PyExc_RecursionError, + "cannot set the recursion limit to %i at " + "the recursion depth %i: the limit is too low", + new_limit, tstate->recursion_depth); + return NULL; + } + Py_SetRecursionLimit(new_limit); Py_INCREF(Py_None); return Py_None; -- cgit v0.12