[PEP 558 - WIP] bpo-30744: Trace hooks no longer reset closure state

Include/descrobject.h

@@ -98,7 +98,14 @@ PyAPI_FUNC(PyObject *) PyDescr_NewWrapper(PyTypeObject *,
 #define PyDescr_IsData(d) (Py_TYPE(d)->tp_descr_set != NULL)
 #endif
 
+/* PyDictProxy should really have its own header/impl pair, but keeping
+ * it here for now... */
+
 PyAPI_FUNC(PyObject *) PyDictProxy_New(PyObject *);
+#ifdef Py_BUILD_CORE
+PyAPI_DATA(PyTypeObject) PyDictProxy_Type;
+#endif
+
 PyAPI_FUNC(PyObject *) PyWrapper_New(PyObject *, PyObject *);
 
 

Include/frameobject.h

@@ -78,6 +78,9 @@ PyAPI_FUNC(void) PyFrame_LocalsToFast(PyFrameObject *, int);
 
 PyAPI_FUNC(int) PyFrame_FastToLocalsWithError(PyFrameObject *f);
 PyAPI_FUNC(void) PyFrame_FastToLocals(PyFrameObject *);
+#ifdef Py_BUILD_CORE
+int _PyFrame_FastToLocalsInternal(PyFrameObject *f, int); /* For sys.settrace */
+#endif
 
 PyAPI_FUNC(int) PyFrame_ClearFreeList(void);
 
@@ -86,6 +89,14 @@ PyAPI_FUNC(void) _PyFrame_DebugMallocStats(FILE *out);
 /* Return the line of code the frame is currently executing. */
 PyAPI_FUNC(int) PyFrame_GetLineNumber(PyFrameObject *);
 
+
+#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03080000
+/* Fast locals proxy for reliable write-through from trace functions */
+PyTypeObject PyFastLocalsProxy_Type;
+#define _PyFastLocalsProxy_CheckExact(self) \
+    (Py_TYPE(self) == &PyFastLocalsProxy_Type)
+#endif
+
 #ifdef __cplusplus
 }
 #endif

Lib/test/test_sys_settrace.py

@@ -5,6 +5,7 @@
 import sys
 import difflib
 import gc
+import textwrap
 from functools import wraps
 import asyncio
 
@@ -1405,6 +1406,149 @@ def gen():
         next(gen())
         output.append(5)
 
+class FrameLocalsTestCase(unittest.TestCase):
+    def setUp(self):
+        self.addCleanup(sys.settrace, sys.gettrace())
+        sys.settrace(None)
+
+    def test_closures_are_not_implicitly_reset_to_previous_state(self):
+        # See https://bugs.python.org/issue30744 for details
+        i_from_generator = []
+        x_from_generator = []
+        x_from_nested_ref = []
+        x_from_nested_locals = []
+        def outer():
+            x = 0
+
+            def update_nested_refs():
+                x_from_nested_ref.append(x)
+                x_from_nested_locals.append(locals()["x"])
+
+            yield update_nested_refs
+            for i in range(1, 10):
+                i_from_generator.append(i)
+                x += 1
+                yield x
+
+        incrementing_generator = outer()
+        update_nested_refs = next(incrementing_generator)
+
+        def tracefunc(frame, event, arg):
+            x_from_generator.append(next(incrementing_generator))
+            return tracefunc
+
+        sys.settrace(tracefunc)
+        try:
+            update_nested_refs()
+            update_nested_refs()
+        finally:
+            sys.settrace(None)
+        self.assertEqual(x_from_generator, i_from_generator)
+        self.assertEqual(x_from_nested_ref, [2, 6])
+        self.assertEqual(x_from_nested_locals, [3, 7])
+
+    def test_locals_writeback_support(self):
+        # To support interactive debuggers, trace functions are expected to
+        # be able to reliably modify function locals. However, this should
+        # NOT enable writebacks via locals() at function scope.
+        #
+        # Note: the sample values have numbers in them so mixing up variable
+        #       names in the checks can't accidentally make the test pass -
+        #       you'd have to get both the name *and* expected number wrong
+        self.maxDiff = None
+        code = textwrap.dedent("""
+            locals()['a_global'] = 'created1' # We expect this to be retained
+            another_global = 'original2'      # Trace func will modify this
+
+            class C:
+                locals()['an_attr'] = 'created3' # We expect this to be retained
+                another_attr = 'original4'       # Trace func will modify this
+            a_class_attribute = C.an_attr
+            another_class_attribute = C.another_attr
+            del C
+
+            def outer():
+                a_nonlocal = 'original5'       # We expect this to be retained
+                another_nonlocal = 'original6' # Trace func will modify this
+                def inner():
+                    nonlocal another_nonlocal
+                    a_local = 'original7'          # We expect this to be retained
+                    another_local = 'original8'    # Trace func will modify this
+                    ns = locals()
+                    ns['a_local'] = 'modified7'    # We expect this to be reverted
+                    ns['a_nonlocal'] = 'modified5' # We expect this to be reverted
+                    ns['a_new_local'] = 'created9' # We expect this to be retained
+                    return a_local, another_local, ns
+                outer_local = 'original10' # Trace func will modify this
+                # Trigger any updates from the inner function & trace function
+                inner_result = inner()
+                outer_result = a_nonlocal, another_nonlocal, outer_local, locals()
+                return outer_result, inner_result
+            outer_result, inner_result = outer()
+            a_nonlocal, another_nonlocal, outer_local, outer_ns = outer_result
+            a_nonlocal_via_ns = outer_ns['a_nonlocal']
+            another_nonlocal_via_ns = outer_ns['another_nonlocal']
+            outer_local_via_ns = outer_ns['outer_local']
+            a_local, another_local, inner_ns = inner_result
+            a_local_via_ns = inner_ns['a_local']
+            a_nonlocal_via_inner_ns = inner_ns['a_nonlocal']
+            another_nonlocal_via_inner_ns = inner_ns['another_nonlocal']
+            another_local_via_ns = inner_ns['another_local']
+            a_new_local_via_ns = inner_ns['a_new_local']
+            del outer, outer_result, outer_ns, inner_result, inner_ns
+            """
+        )
+        def tracefunc(frame, event, arg):
+            if event == "return":
+                # We leave any state manipulation to the very end
+                ns = frame.f_locals
+                co_name = frame.f_code.co_name
+                if co_name == "C":
+                    # Modify class attributes
+                    ns["another_attr"] = "modified4"
+                elif co_name == "inner":
+                    # Modify local and nonlocal variables
+                    ns["another_nonlocal"] = "modified6"
+                    ns["another_local"] = "modified8"
+                    outer_ns = frame.f_back.f_locals
+                    outer_ns["outer_local"] = "modified10"
+                elif co_name == "<module>":
+                    # Modify globals
+                    ns["another_global"] = "modified2"
+            return tracefunc
+        actual_ns = {}
+        sys.settrace(tracefunc)
+        try:
+            exec(code, actual_ns)
+        finally:
+            sys.settrace(None)
+        for k in list(actual_ns.keys()):
+            if k.startswith("_"):
+                del actual_ns[k]
+        # CURRENT FAILURE STATUS:
+        # - trace functions mutating state as expected
+        # - proxy is incorrectly being returned by locals(), so that also mutates
+        expected_ns = {
+            "a_global": "created1",
+            "another_global": "modified2",
+            "a_class_attribute": "created3",
+            "another_class_attribute": "modified4",
+            "a_nonlocal": "original5",
+            "a_nonlocal_via_ns": "original5",
+            "a_nonlocal_via_inner_ns": "original5",
+            "another_nonlocal": "modified6",
+            "another_nonlocal_via_ns": "modified6",
+            "another_nonlocal_via_inner_ns": "modified6",
+            "a_local": "original7",
+            "a_local_via_ns": "original7",
+            "another_local": "modified8",
+            "another_local_via_ns": "modified8",
+            "a_new_local_via_ns": "created9",
+            "outer_local": "modified10",
+            "outer_local_via_ns": "modified10",
+        }
+        self.assertEqual(actual_ns, expected_ns)
+
 
 if __name__ == "__main__":
     unittest.main()

Objects/descrobject.c

@@ -1004,6 +1004,7 @@ PyDictProxy_New(PyObject *mapping)
 }
 
 
+
 /* --- Wrapper object for "slot" methods --- */
 
 /* This has no reason to be in this file except that adding new files is a
@@ -1569,7 +1570,9 @@ PyTypeObject PyDictProxy_Type = {
     PyObject_GenericGetAttr,                    /* tp_getattro */
     0,                                          /* tp_setattro */
     0,                                          /* tp_as_buffer */
-    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */
+    Py_TPFLAGS_DEFAULT |
+    Py_TPFLAGS_HAVE_GC |
+    Py_TPFLAGS_BASETYPE,                        /* tp_flags */
     0,                                          /* tp_doc */
     mappingproxy_traverse,                      /* tp_traverse */
     0,                                          /* tp_clear */