Skip to content

[cpyrt] Do not decref a constructor result on Windows - #70

Merged
aaronj0 merged 1 commit into
compiler-research:mainfrom
aaronj0:no-decref-ctor-result-windows
Sep 3, 2026
Merged

[cpyrt] Do not decref a constructor result on Windows#70
aaronj0 merged 1 commit into
compiler-research:mainfrom
aaronj0:no-decref-ctor-result-windows

Conversation

@aaronj0

@aaronj0 aaronj0 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

ExecuteFast() ends with a Windows-only block that drops the result when a Python exception is pending. It is shared by every CPPMethod subclass, including CPPConstructor, whose executor returns the address of the new C++ object cast to PyObject*; decref'ing that corrupts the heap. Ask the method whether its executor hands back a real PyObject* first. The object leaks instead, on the error path of a call that is already failing.

ExecuteFast() ends with a Windows-only block that drops the result when
a Python exception is pending. It is shared by every CPPMethod subclass,
including CPPConstructor, whose executor returns the address of the new
C++ object cast to PyObject*; decref'ing that corrupts the heap. Ask the
method whether its executor hands back a real PyObject* first. The
object leaks instead, on the error path of a call that is already
failing.
@aaronj0
aaronj0 force-pushed the no-decref-ctor-result-windows branch from 48f2f30 to c35a4b9 Compare September 2, 2026 11:10
@aaronj0
aaronj0 requested a review from guitargeek September 2, 2026 11:37

@guitargeek guitargeek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! The undefined behavior fixes are indeed not controversial.

@aaronj0
aaronj0 merged commit 9a0217a into compiler-research:main Sep 3, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants