sys.setrecursionlimit() now raises RecursionError

Issue python#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.

Author: vstinner

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)
 

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;\

Lib/test/test_sys.py

@@ -201,22 +201,60 @@ def test_recursionlimit_recovery(self):
         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.

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.
 

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 */
 };
 

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;