address review comments

1. define wrapping code at the module level 2. increase recursion limit delta from 25 to 30 3. add a note to the wrapped functions' doc-strings 4. extract the recursion limit delta to a variable 5. rework test and add another one
python · taleinat · Jul 6, 2019 · Jun 10, 2019 · Jun 10, 2019 · Jun 10, 2019
commit af3ee2b156162bfce1f09614168e7b54613befa7
diff --git a/Lib/idlelib/idle_test/test_run.py b/Lib/idlelib/idle_test/test_run.py
@@ -6,6 +6,8 @@
 from test.support import captured_stderr
 
 import io
+import sys
+
 
 class RunTest(unittest.TestCase):
 
@@ -260,34 +262,27 @@ def test_close(self):
         self.assertRaises(TypeError, f.close, 1)
 
 
-class TestExecutive(unittest.TestCase):
-    def setUp(self):
-        patcher1 = mock.patch('idlelib.run.autocomplete')
-        patcher1.start()
-        self.addCleanup(patcher1.stop)
-
-        patcher2 = mock.patch('idlelib.run.calltip')
-        patcher2.start()
-        self.addCleanup(patcher2.stop)
-
-    def test_sys_recursionlimit(self):
-        rpchandler = mock.Mock()
-        executor = run.Executive(rpchandler)
-
-        if 'sys' not in executor.locals:
-            exec('import sys', executor.locals)
-            self.addCleanup(exec, 'del sys', executor.locals)
-        executor_sys = executor.locals['sys']
+class TestSysRecursionLimitWrappers(unittest.TestCase):
+    def test_roundtrip(self):
+        run.install_recursionlimit_wrappers()
+        self.addCleanup(run.uninstall_recursionlimit_wrappers)
 
         # check that setting the recursion limit works
-        orig_reclimit = executor_sys.getrecursionlimit()
-        self.addCleanup(executor_sys.setrecursionlimit, orig_reclimit)
-        executor_sys.setrecursionlimit(orig_reclimit + 3)
+        orig_reclimit = sys.getrecursionlimit()
+        self.addCleanup(sys.setrecursionlimit, orig_reclimit)
+        sys.setrecursionlimit(orig_reclimit + 3)
 
         # check that the new limit is returned by sys.getrecursionlimit()
-        new_reclimit = executor_sys.getrecursionlimit()
+        new_reclimit = sys.getrecursionlimit()
         self.assertEqual(new_reclimit, orig_reclimit + 3)
 
+    def test_default_recursion_limit_preserved(self):
+        orig_reclimit = sys.getrecursionlimit()
+        run.install_recursionlimit_wrappers()
+        self.addCleanup(run.uninstall_recursionlimit_wrappers)
+        new_reclimit = sys.getrecursionlimit()
+        self.assertEqual(new_reclimit, orig_reclimit)
+
 
 if __name__ == '__main__':
     unittest.main(verbosity=2)
diff --git a/Lib/idlelib/run.py b/Lib/idlelib/run.py
@@ -4,6 +4,7 @@
 f'''{sys.executable} -c "__import__('idlelib.run').run.main()"'''
 '.run' is needed because __import__ returns idlelib, not idlelib.run.
 """
+import functools
 import io
 import linecache
 import queue
@@ -304,6 +305,60 @@ def fix_scaling(root):
                 font['size'] = round(-0.75*size)
 
 
+RECURSIONLIMIT_DELTA = 30
+def install_recursionlimit_wrappers():
+    """Install wrappers to always add 30 to the recursion limit."""
+    # see: bpo-26806
+
+    @functools.wraps(sys.setrecursionlimit)
+    def setrecursionlimit(*args, **kwargs):
+        # mimic the original sys.setrecursionlimit()'s input handling
+        if kwargs:
+            raise TypeError(
+                "setrecursionlimit() takes no keyword arguments")
+        try:
+            limit, = args
+        except ValueError:
+            raise TypeError("setrecursionlimit() takes exactly one "
+                            "argument ({} given)".format(len(args)))
+        if not limit > 0:
+            raise ValueError(
+                "recursion limit must be greater or equal than 1")
+
+        return setrecursionlimit.__wrapped__(limit + RECURSIONLIMIT_DELTA)
+
+    setrecursionlimit.__doc__ += "\n\n" + textwrap.fill(textwrap.dedent(f"""\
+        This IDLE wrapper adds {RECURSIONLIMIT_DELTA} to prevent possible
+        uninterruptible loops.
+        """).strip())
+
+    @functools.wraps(sys.getrecursionlimit)
+    def getrecursionlimit():
+        return getrecursionlimit.__wrapped__() - RECURSIONLIMIT_DELTA
+
+    getrecursionlimit.__doc__ += "\n\n" + textwrap.fill(textwrap.dedent(f"""\
+        This IDLE wrapper subtracts {RECURSIONLIMIT_DELTA} to compensate for
+        the {RECURSIONLIMIT_DELTA} IDLE adds when setting the limit.
+        """).strip())
+
+    # add the delta to the default recursion limit, to compensate
+    sys.setrecursionlimit(sys.getrecursionlimit() + RECURSIONLIMIT_DELTA)
+
+    sys.setrecursionlimit = setrecursionlimit
+    sys.getrecursionlimit = getrecursionlimit
+
+
+def uninstall_recursionlimit_wrappers():
+    """Uninstall the recursion limit wrappers from the sys module."""
+    if (
+            getattr(sys.setrecursionlimit, '__wrapped__') and
+            getattr(sys.getrecursionlimit, '__wrapped__')
+    ):
+        sys.setrecursionlimit = sys.setrecursionlimit.__wrapped__
+        sys.getrecursionlimit = sys.getrecursionlimit.__wrapped__
+        sys.setrecursionlimit(sys.getrecursionlimit() - RECURSIONLIMIT_DELTA)
+
+
 class MyRPCServer(rpc.RPCServer):
 
     def handle_error(self, request, client_address):
@@ -447,6 +502,8 @@ def handle(self):
         # sys.stdin gets changed from within IDLE's shell. See issue17838.
         self._keep_stdin = sys.stdin
 
+        install_recursionlimit_wrappers()
+
         self.interp = self.get_remote_proxy("interp")
         rpc.RPCHandler.getresponse(self, myseq=None, wait=0.05)
 
@@ -475,50 +532,6 @@ def __init__(self, rpchandler):
         self.calltip = calltip.Calltip()
         self.autocomplete = autocomplete.AutoComplete()
 
-        self._wrap_sys_setrecursionlimit()
-
-    def _wrap_sys_setrecursionlimit(self):
-        "transparently always add 25 to the recursion limit"
-        # see: bpo-26806
-        code = textwrap.dedent("""\
-            import sys
-            from functools import wraps
-
-            # add the delta to the default recursion limit, to compensate
-            sys.setrecursionlimit(sys.getrecursionlimit() + 25)
-
-            @wraps(sys.setrecursionlimit)
-            def setrecursionlimit(*args, **kwargs):
-                # mimic the original sys.setrecursionlimit()'s input handling
-                if kwargs:
-                    raise TypeError(
-                        "setrecursionlimit() takes no keyword arguments")
-                try:
-                    limit, = args
-                except ValueError:
-                    raise TypeError("setrecursionlimit() takes exactly one "
-                                    "argument ({} given)".format(len(args)))
-                if not limit > 0:
-                    raise ValueError(
-                        "recursion limit must be greater or equal than 1")
-
-                import sys
-                return sys.setrecursionlimit.__wrapped__(limit + 25)
-            sys.setrecursionlimit = setrecursionlimit
-            del setrecursionlimit
-
-            @wraps(sys.getrecursionlimit)
-            def getrecursionlimit():
-                import sys
-                return sys.getrecursionlimit.__wrapped__() - 25
-            sys.getrecursionlimit = getrecursionlimit
-            del getrecursionlimit
-
-            del sys
-            del wraps
-            """)
-        exec(code, self.locals)
-
     def runcode(self, code):
         global interruptable
         try: