* Moving null-check of tp_finalize from runtime to compile time.

* Calling PyObject_GC_IsFinalized(...) to ensure tp_finalize is called exactly once.

Author: advait-dixit

mypyc/codegen/emitclass.py

@@ -311,7 +311,7 @@ def emit_line() -> None:
         emit_line()
         generate_clear_for_class(cl, clear_name, emitter)
         emit_line()
-        generate_dealloc_for_class(cl, dealloc_name, clear_name, emitter)
+        generate_dealloc_for_class(cl, dealloc_name, clear_name, bool(del_method), emitter)
         emit_line()
         if del_method:
             generate_finalize_for_class(del_method, finalize_name, emitter)
@@ -782,14 +782,15 @@ def generate_clear_for_class(cl: ClassIR, func_name: str, emitter: Emitter) -> N
 
 
 def generate_dealloc_for_class(
-    cl: ClassIR, dealloc_func_name: str, clear_func_name: str, emitter: Emitter
+    cl: ClassIR, dealloc_func_name: str, clear_func_name: str, has_tp_finalize: bool, emitter: Emitter
 ) -> None:
     emitter.emit_line("static void")
     emitter.emit_line(f"{dealloc_func_name}({cl.struct_name(emitter.names)} *self)")
     emitter.emit_line("{")
-    emitter.emit_line("if (Py_TYPE(self)->tp_finalize) {")
-    emitter.emit_line("    Py_TYPE(self)->tp_finalize((PyObject *)self);")
-    emitter.emit_line("}")
+    if has_tp_finalize:
+        emitter.emit_line("if (!PyObject_GC_IsFinalized((PyObject *)self)) {")
+        emitter.emit_line("Py_TYPE(self)->tp_finalize((PyObject *)self);")
+        emitter.emit_line("}")
     emitter.emit_line("PyObject_GC_UnTrack(self);")
     # The trashcan is needed to handle deep recursive deallocations
     emitter.emit_line(f"CPy_TRASHCAN_BEGIN(self, {dealloc_func_name})")
@@ -805,13 +806,18 @@ def generate_finalize_for_class(
     emitter.emit_line("static void")
     emitter.emit_line(f"{finalize_func_name}(PyObject *self)")
     emitter.emit_line("{")
+    emitter.emit_line("PyObject *type, *value, *traceback;")
+    emitter.emit_line("PyErr_Fetch(&type, &value, &traceback);")
     emitter.emit_line(
         "{}{}{}(self);".format(
             emitter.get_group_prefix(del_method.decl),
             NATIVE_PREFIX,
             del_method.cname(emitter.names),
         )
     )
+    emitter.emit_line("if (type != NULL) {")
+    emitter.emit_line("PyErr_Restore(type, value, traceback);")
+    emitter.emit_line("}")
     emitter.emit_line("}")
 
 

mypyc/test-data/run-classes.test

@@ -2737,11 +2737,47 @@ class C(B):
         print("deleting C...")
         super().__del__()
 
+class D(A):
+    pass
+
 [file driver.py]
 import native
 native.C()
+native.D()
 
 [out]
 deleting C...
 deleting B...
 deleting A...
+deleting A...
+
+[case testDelCircular]
+import dataclasses
+import typing
+
+i: int = 1
+
+@dataclasses.dataclass
+class C:
+    var: typing.Optional["C"] = dataclasses.field(default=None)
+
+    def __del__(self):
+        global i
+        print(f"deleting C{i}...")
+        i = i + 1
+
+[file driver.py]
+import native
+import gc
+
+c1 = native.C()
+c2 = native.C()
+c1.var = c2
+c2.var = c1
+del c1
+del c2
+gc.collect()
+
+[out]
+deleting C1...
+deleting C2...