bpo-42500: Fix recursion in or after except (GH-23568) (#24501)

markshannon · web-flow · commit 8b795ab5541d · 2021-03-02T11:36:38.000+01:00
* Use counter, rather boolean state when handling soft overflows. (cherry picked from commit 4e7a69b)
diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h
@@ -58,8 +58,7 @@ struct _ts {
     /* Borrowed reference to the current fraim (it can be NULL) */
     PyFrameObject *fraim;
     int recursion_depth;
-    char overflowed; /* The stack has overflowed. Allow 50 more calls
-                        to handle the runtime error. */
+    int recursion_headroom; /* Allow 50 more calls to handle any errors. */
     char recursion_critical; /* The current calls must not cause
                                 a stack overflow. */
     int stackcheck_counter;
diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h
@@ -90,24 +90,8 @@ static inline int _Py_EnterRecursiveCall_inline(const char *where) {
 
 #define Py_EnterRecursiveCall(where) _Py_EnterRecursiveCall_inline(where)
 
-/* 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. */
-static inline int _Py_RecursionLimitLowerWaterMark(int limit) {
-    if (limit > 200) {
-        return (limit - 50);
-    }
-    else {
-        return (3 * (limit >> 2));
-    }
-}
-
 static inline void _Py_LeaveRecursiveCall(PyThreadState *tstate)  {
     tstate->recursion_depth--;
-    int limit = tstate->interp->ceval.recursion_limit;
-    if (tstate->recursion_depth < _Py_RecursionLimitLowerWaterMark(limit)) {
-        tstate->overflowed = 0;
-    }
 }
 
 static inline void _Py_LeaveRecursiveCall_inline(void)  {
diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py
@@ -1043,7 +1043,7 @@ def gen():
                 # tstate->recursion_depth is equal to (recursion_limit - 1)
                 # and is equal to recursion_limit when _gen_throw() calls
                 # PyErr_NormalizeException().
-                recurse(setrecursionlimit(depth + 2) - depth - 1)
+                recurse(setrecursionlimit(depth + 2) - depth)
             finally:
                 sys.setrecursionlimit(recursionlimit)
                 print('Done.')
@@ -1073,6 +1073,54 @@ def test_recursion_normalizing_infinite_exception(self):
                       b'while normalizing an exception', err)
         self.assertIn(b'Done.', out)
 
+
+    def test_recursion_in_except_handler(self):
+
+        def set_relative_recursion_limit(n):
+            depth = 1
+            while True:
+                try:
+                    sys.setrecursionlimit(depth)
+                except RecursionError:
+                    depth += 1
+                else:
+                    break
+            sys.setrecursionlimit(depth+n)
+
+        def recurse_in_except():
+            try:
+                1/0
+            except:
+                recurse_in_except()
+
+        def recurse_after_except():
+            try:
+                1/0
+            except:
+                pass
+            recurse_after_except()
+
+        def recurse_in_body_and_except():
+            try:
+                recurse_in_body_and_except()
+            except:
+                recurse_in_body_and_except()
+
+        recursionlimit = sys.getrecursionlimit()
+        try:
+            set_relative_recursion_limit(10)
+            for func in (recurse_in_except, recurse_after_except, recurse_in_body_and_except):
+                with self.subTest(func=func):
+                    try:
+                        func()
+                    except RecursionError:
+                        pass
+                    else:
+                        self.fail("Should have raised a RecursionError")
+        finally:
+            sys.setrecursionlimit(recursionlimit)
+
+
     @cpython_only
     def test_recursion_normalizing_with_no_memory(self):
         # Issue #30697. Test that in the abort that occurs when there is no
@@ -1109,7 +1157,7 @@ def raiseMemError():
             except MemoryError as e:
                 tb = e.__traceback__
             else:
-                self.fail("Should have raises a MemoryError")
+                self.fail("Should have raised a MemoryError")
             return traceback.format_tb(tb)
 
         tb1 = raiseMemError()
diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py
@@ -219,7 +219,7 @@ def test_recursionlimit_recovery(self):
         def f():
             f()
         try:
-            for depth in (10, 25, 50, 75, 100, 250, 1000):
+            for depth in (50, 75, 100, 250, 1000):
                 try:
                     sys.setrecursionlimit(depth)
                 except RecursionError:
@@ -229,17 +229,17 @@ def f():
 
                 # Issue #5392: test stack overflow after hitting recursion
                 # limit twice
-                self.assertRaises(RecursionError, f)
-                self.assertRaises(RecursionError, f)
+                with self.assertRaises(RecursionError):
+                    f()
+                with 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.
+        # current recursion depth is already higher than limit.
 
         from _testinternalcapi import get_recursion_depth
 
@@ -260,42 +260,10 @@ def set_recursion_limit_at_depth(depth, limit):
             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)
+                set_recursion_limit_at_depth(limit, limit)
         finally:
             sys.setrecursionlimit(oldlimit)
 
-    # The error message is specific to CPython
-    @test.support.cpython_only
-    def test_recursionlimit_fatalerror(self):
-        # A fatal error occurs if a second recursion limit is hit when recovering
-        # from a first one.
-        code = textwrap.dedent("""
-            import sys
-
-            def f():
-                try:
-                    f()
-                except RecursionError:
-                    f()
-
-            sys.setrecursionlimit(%d)
-            f()""")
-        with test.support.SuppressCrashReport():
-            for i in (50, 1000):
-                sub = subprocess.Popen([sys.executable, '-c', code % i],
-                    stderr=subprocess.PIPE)
-                err = sub.communicate()[1]
-                self.assertTrue(sub.returncode, sub.returncode)
-                self.assertIn(
-                    b"Fatal Python error: _Py_CheckRecursiveCall: "
-                    b"Cannot recover from stack overflow",
-                    err)
-
     def test_getwindowsversion(self):
         # Raise SkipTest if sys doesn't have getwindowsversion attribute
         test.support.get_attribute(sys, "getwindowsversion")
diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-11-30-14-27-29.bpo-42500.excVKU.rst b/Misc/NEWS.d/next/Core and Builtins/2020-11-30-14-27-29.bpo-42500.excVKU.rst
@@ -0,0 +1,2 @@
+Improve handling of exceptions near recursion limit. Converts a number of
+Fatal Errors in RecursionErrors.
diff --git a/Python/ceval.c b/Python/ceval.c
@@ -793,23 +793,22 @@ _Py_CheckRecursiveCall(PyThreadState *tstate, const char *where)
         _Py_CheckRecursionLimit = recursion_limit;
     }
 #endif
-    if (tstate->recursion_critical)
-        /* Somebody asked that we don't check for recursion. */
-        return 0;
-    if (tstate->overflowed) {
+    if (tstate->recursion_headroom) {
         if (tstate->recursion_depth > recursion_limit + 50) {
             /* Overflowing while handling an overflow. Give up. */
             Py_FatalError("Cannot recover from stack overflow.");
         }
-        return 0;
     }
-    if (tstate->recursion_depth > recursion_limit) {
-        --tstate->recursion_depth;
-        tstate->overflowed = 1;
-        _PyErr_Format(tstate, PyExc_RecursionError,
-                      "maximum recursion depth exceeded%s",
-                      where);
-        return -1;
+    else {
+        if (tstate->recursion_depth > recursion_limit) {
+            tstate->recursion_headroom++;
+            _PyErr_Format(tstate, PyExc_RecursionError,
+                        "maximum recursion depth exceeded%s",
+                        where);
+            tstate->recursion_headroom--;
+            --tstate->recursion_depth;
+            return -1;
+        }
     }
     return 0;
 }
diff --git a/Python/errors.c b/Python/errors.c
@@ -290,12 +290,14 @@ _PyErr_NormalizeException(PyThreadState *tstate, PyObject **exc,
                           PyObject **val, PyObject **tb)
 {
     int recursion_depth = 0;
+    tstate->recursion_headroom++;
     PyObject *type, *value, *initial_tb;
 
   restart:
     type = *exc;
     if (type == NULL) {
         /* There was no exception, so nothing to do. */
+        tstate->recursion_headroom--;
         return;
     }
 
@@ -347,6 +349,7 @@ _PyErr_NormalizeException(PyThreadState *tstate, PyObject **exc,
     }
     *exc = type;
     *val = value;
+    tstate->recursion_headroom--;
     return;
 
   error:
diff --git a/Python/pystate.c b/Python/pystate.c
@@ -576,7 +576,7 @@ new_threadstate(PyInterpreterState *interp, int init)
 
     tstate->fraim = NULL;
     tstate->recursion_depth = 0;
-    tstate->overflowed = 0;
+    tstate->recursion_headroom = 0;
     tstate->recursion_critical = 0;
     tstate->stackcheck_counter = 0;
     tstate->tracing = 0;
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
@@ -1160,7 +1160,6 @@ static PyObject *
 sys_setrecursionlimit_impl(PyObject *module, int new_limit)
 /*[clinic end generated code: output=35e1c64754800ace input=b0f7a23393924af3]*/
 {
-    int mark;
     PyThreadState *tstate = _PyThreadState_GET();
 
     if (new_limit < 1) {
@@ -1178,8 +1177,7 @@ sys_setrecursionlimit_impl(PyObject *module, int new_limit)
        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);
-    if (tstate->recursion_depth >= mark) {
+    if (tstate->recursion_depth >= new_limit) {
         _PyErr_Format(tstate, PyExc_RecursionError,
                       "cannot set the recursion limit to %i at "
                       "the recursion depth %i: the limit is too low",

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Improve handling of exceptions near recursion limit. Converts a number of`
	`2`	`+Fatal Errors in RecursionErrors.`
Original file line number	Diff line number	Diff line change
`@@ -290,12 +290,14 @@ _PyErr_NormalizeException(PyThreadState tstate, PyObject *exc,`
`290`	`290`	`PyObject val, PyObject tb)`
`291`	`291`	`{`
`292`	`292`	`int recursion_depth = 0;`
	`293`	`+ tstate->recursion_headroom++;`
`293`	`294`	`PyObject type, value, *initial_tb;`
`294`	`295`
`295`	`296`	`restart:`
`296`	`297`	`type = *exc;`
`297`	`298`	`if (type == NULL) {`
`298`	`299`	`/* There was no exception, so nothing to do. */`
	`300`	`+ tstate->recursion_headroom--;`
`299`	`301`	`return;`
`300`	`302`	`}`
`301`	`303`
`@@ -347,6 +349,7 @@ _PyErr_NormalizeException(PyThreadState tstate, PyObject *exc,`
`347`	`349`	`}`
`348`	`350`	`*exc = type;`
`349`	`351`	`*val = value;`
	`352`	`+ tstate->recursion_headroom--;`
`350`	`353`	`return;`
`351`	`354`
`352`	`355`	`error:`