Skip to content

[mypyc] Calling tp_finalize from tp_dealloc. #18519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Feb 14, 2025
Merged
53 changes: 51 additions & 2 deletions mypyc/codegen/emitclass.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ def generate_class(cl: ClassIR, module: str, emitter: Emitter) -> None:

setup_name = f"{name_prefix}_setup"
new_name = f"{name_prefix}_new"
finalize_name = f"{name_prefix}_finalize"
members_name = f"{name_prefix}_members"
getseters_name = f"{name_prefix}_getseters"
vtable_name = f"{name_prefix}_vtable"
Expand All @@ -217,6 +218,10 @@ def generate_class(cl: ClassIR, module: str, emitter: Emitter) -> None:
fields["tp_dealloc"] = f"(destructor){name_prefix}_dealloc"
fields["tp_traverse"] = f"(traverseproc){name_prefix}_traverse"
fields["tp_clear"] = f"(inquiry){name_prefix}_clear"
# Populate .tp_finalize and generate a finalize method only if __del__ is defined for this class.
del_method = next((e.method for e in cl.vtable_entries if e.name == "__del__"), None)
if del_method:
fields["tp_finalize"] = f"(destructor){finalize_name}"
if needs_getseters:
fields["tp_getset"] = getseters_name
fields["tp_methods"] = methods_name
Expand Down Expand Up @@ -297,8 +302,11 @@ 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)
emit_line()

if cl.allow_interpreted_subclasses:
shadow_vtable_name: str | None = generate_vtables(
Expand Down Expand Up @@ -765,11 +773,19 @@ 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("{")
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})")
Expand All @@ -779,6 +795,39 @@ def generate_dealloc_for_class(
emitter.emit_line("}")


def generate_finalize_for_class(
del_method: FuncIR, finalize_func_name: str, emitter: Emitter
) -> None:
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(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we preserve the current exception status, as suggested in the docs: https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_finalize

Copy link
Contributor Author

@advait-dixit advait-dixit Jan 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

emitter.get_group_prefix(del_method.decl),
NATIVE_PREFIX,
del_method.cname(emitter.names),
)
)
emitter.emit_line("if (PyErr_Occurred() != NULL) {")
emitter.emit_line('PyObject *del_str = PyUnicode_FromString("__del__");')
emitter.emit_line(
"PyObject *del_method = (del_str == NULL) ? NULL : _PyType_Lookup(Py_TYPE(self), del_str);"
)
# CPython interpreter uses PyErr_WriteUnraisable: https://docs.python.org/3/c-api/exceptions.html#c.PyErr_WriteUnraisable
# However, the message is slightly different due to the way mypyc compiles classes.
# CPython interpreter prints: Exception ignored in: <function F.__del__ at 0x100aed940>
# mypyc prints: Exception ignored in: <slot wrapper '__del__' of 'F' objects>
emitter.emit_line("PyErr_WriteUnraisable(del_method);")
emitter.emit_line("Py_XDECREF(del_method);")
emitter.emit_line("Py_XDECREF(del_str);")
emitter.emit_line("}")
# PyErr_Restore also clears exception raised in __del__.
emitter.emit_line("PyErr_Restore(type, value, traceback);")
emitter.emit_line("}")


def generate_methods_table(cl: ClassIR, name: str, emitter: Emitter) -> None:
emitter.emit_line(f"static PyMethodDef {name}[] = {{")
for fn in cl.methods.values():
Expand Down
81 changes: 81 additions & 0 deletions mypyc/test-data/run-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -2748,3 +2748,84 @@ def test_function():
assert(isinstance(d.bitems, BackwardDefinedClass))
assert(isinstance(d.fitem, ForwardDefinedClass))
assert(isinstance(d.fitems, ForwardDefinedClass))

[case testDel]
class A:
def __del__(self):
print("deleting A...")

class B:
def __del__(self):
print("deleting B...")

class C(B):
def __init__(self):
self.a = A()

def __del__(self):
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...

[case testDelException]
# The error message in the expected output of this test does not match CPython's error message due to the way mypyc compiles Python classes. If the error message is fixed, the expected output of this test will also change.
class F:
def __del__(self):
if True:
raise Exception("e2")

[file driver.py]
import native
f = native.F()
del f

[out]
Exception ignored in: <slot wrapper '__del__' of 'F' objects>
Traceback (most recent call last):
File "native.py", line 5, in __del__
raise Exception("e2")
Exception: e2