Skip to content
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.

Sign up for GitHub

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

Stop using ctypes for traceback mutation #3203

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+6 −114
Open

Stop using ctypes for traceback mutation #3203

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f1d683
Kill ctypes magic for traceback mutation
A5rocks Feb 9, 2025
30647f8
Add a newsfragment
A5rocks Feb 9, 2025
e3ed59d
Update outdated comments
A5rocks Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/405.misc.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Stop using ctypes to mutate tracebacks for ``strict_exception_groups=False``'s exception collapsing.
113 changes: 4 additions & 109 deletions src/trio/_core/_concat_tb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,118 +1,11 @@
from __future__ import annotations

from typing import TYPE_CHECKING, ClassVar, cast

if TYPE_CHECKING:
from types import TracebackType
from types import TracebackType

################################################################
# concat_tb
################################################################

A5rocks marked this conversation as resolved.
Show resolved Hide resolved
# We need to compute a new traceback that is the concatenation of two existing
# tracebacks. This requires copying the entries in 'head' and then pointing
# the final tb_next to 'tail'.
#
# NB: 'tail' might be None, which requires some special handling in the ctypes
# version.
#
# The complication here is that Python doesn't actually support copying or
# modifying traceback objects, so we have to get creative...
#
# On CPython, we use ctypes. On PyPy, we use "transparent proxies".
#
# Jinja2 is a useful source of inspiration:
# https://github.com/pallets/jinja/blob/main/src/jinja2/debug.py

try:
import tputil
except ImportError:
# ctypes it is
# How to handle refcounting? I don't want to use ctypes.py_object because
# I don't understand or trust it, and I don't want to use
# ctypes.pythonapi.Py_{Inc,Dec}Ref because we might clash with user code
# that also tries to use them but with different types. So private _ctypes
# APIs it is!
import _ctypes
import ctypes

class CTraceback(ctypes.Structure):
_fields_: ClassVar = [
("PyObject_HEAD", ctypes.c_byte * object().__sizeof__()),
("tb_next", ctypes.c_void_p),
("tb_frame", ctypes.c_void_p),
("tb_lasti", ctypes.c_int),
("tb_lineno", ctypes.c_int),
]

def copy_tb(base_tb: TracebackType, tb_next: TracebackType | None) -> TracebackType:
# TracebackType has no public constructor, so allocate one the hard way
try:
raise ValueError
except ValueError as exc:
new_tb = exc.__traceback__
assert new_tb is not None
c_new_tb = CTraceback.from_address(id(new_tb))

# At the C level, tb_next either points to the next traceback or is
# NULL. c_void_p and the .tb_next accessor both convert NULL to None,
# but we shouldn't DECREF None just because we assigned to a NULL
# pointer! Here we know that our new traceback has only 1 frame in it,
# so we can assume the tb_next field is NULL.
assert c_new_tb.tb_next is None
# If tb_next is None, then we want to set c_new_tb.tb_next to NULL,
# which it already is, so we're done. Otherwise, we have to actually
# do some work:
if tb_next is not None:
_ctypes.Py_INCREF(tb_next)
c_new_tb.tb_next = id(tb_next)

assert c_new_tb.tb_frame is not None
_ctypes.Py_INCREF(base_tb.tb_frame)
old_tb_frame = new_tb.tb_frame
c_new_tb.tb_frame = id(base_tb.tb_frame)
_ctypes.Py_DECREF(old_tb_frame)

c_new_tb.tb_lasti = base_tb.tb_lasti
c_new_tb.tb_lineno = base_tb.tb_lineno

try:
return new_tb
finally:
# delete references from locals to avoid creating cycles
# see test_cancel_scope_exit_doesnt_create_cyclic_garbage
del new_tb, old_tb_frame

else:
# http://doc.pypy.org/en/latest/objspace-proxies.html
def copy_tb(base_tb: TracebackType, tb_next: TracebackType | None) -> TracebackType:
# tputil.ProxyOperation is PyPy-only, and there's no way to specify
# cpython/pypy in current type checkers.
def controller( # type: ignore[no-any-unimported]
operation: tputil.ProxyOperation,
) -> TracebackType | None:
# Rationale for pragma: I looked fairly carefully and tried a few
# things, and AFAICT it's not actually possible to get any
# 'opname' that isn't __getattr__ or __getattribute__. So there's
# no missing test we could add, and no value in coverage nagging
# us about adding one.
if (
operation.opname
in {
"__getattribute__",
"__getattr__",
}
and operation.args[0] == "tb_next"
) or TYPE_CHECKING: # pragma: no cover
return tb_next
# Delegate is reverting to original behaviour
return operation.delegate() # type: ignore[no-any-return]

return cast(
"TracebackType",
tputil.make_proxy(controller, type(base_tb), base_tb),
) # Returns proxy to traceback


# this is used for collapsing single-exception ExceptionGroups when using
# `strict_exception_groups=False`. Once that is retired this function and its helper can
A5rocks marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -131,5 +24,7 @@ def concat_tb(
pointer = pointer.tb_next
current_head = tail
for head_tb in reversed(head_tbs):
current_head = copy_tb(head_tb, tb_next=current_head)
current_head = TracebackType(
current_head, head_tb.tb_frame, head_tb.tb_lasti, head_tb.tb_lineno
)
return current_head