Skip to content

[CPyCppyy] Do not decref a constructor result on Windows - #21157

Merged
guitargeek merged 1 commit into
root-project:masterfrom
guitargeek:python_dev
Aug 9, 2026
Merged

[CPyCppyy] Do not decref a constructor result on Windows#21157
guitargeek merged 1 commit into
root-project:masterfrom
guitargeek:python_dev

Conversation

@guitargeek

@guitargeek guitargeek commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

CPPMethod::ExecuteFast() ends with a Windows-only block that turns a
result returned with a pending Python exception into a failure, to cover
the PyException throw not propagating there:

#ifdef _WIN32
    if (PyErr_Occurred()) {
        Py_XDECREF(result);
        result = nullptr;
    }
#endif

ExecuteFast() is shared by every CPPMethod subclass, including
CPPConstructor, whose executor is ConstructorExecutor. That one does not
return a PyObject* at all: it returns the address of the newly
constructed C++ object, cast to PyObject* for CPPConstructor::Call() to
unpack. Decref'ing that address treats arbitrary C++ object memory as a
PyObject and corrupts the heap.

Ask the method whether its executor hands back a real PyObject* before
dropping a reference. The object is leaked instead, which is acceptable
on the error path of a call that is about to be reported as failed.

🤖 Done with the help of AI

@guitargeek guitargeek self-assigned this Feb 4, 2026
@guitargeek
guitargeek requested a review from bellenot as a code owner February 4, 2026 13:04
@guitargeek
guitargeek force-pushed the python_dev branch 2 times, most recently from eb13c60 to 88c07d3 Compare February 4, 2026 13:24
@github-actions

github-actions Bot commented Feb 4, 2026

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 16h 55m 26s ⏱️
 3 857 tests  3 857 ✅ 0 💤 0 ❌
78 561 runs  78 561 ✅ 0 💤 0 ❌

Results for commit 03753f1.

♻️ This comment has been updated with latest results.

@guitargeek
guitargeek requested a review from vepadulano as a code owner March 23, 2026 20:24
@guitargeek
guitargeek force-pushed the python_dev branch 2 times, most recently from de3e2e9 to 0701b48 Compare March 30, 2026 20:13
@guitargeek
guitargeek requested a review from couet as a code owner May 5, 2026 09:30
@guitargeek
guitargeek force-pushed the python_dev branch 2 times, most recently from 331f2d2 to 073158e Compare August 7, 2026 09:03

@aaronj0 aaronj0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes on the cppyy side LGTM!

CPPMethod::ExecuteFast() ends with a Windows-only block that turns a
result returned with a pending Python exception into a failure, to cover
the PyException throw not propagating there:

    #ifdef _WIN32
        if (PyErr_Occurred()) {
            Py_XDECREF(result);
            result = nullptr;
        }
    #endif

ExecuteFast() is shared by every CPPMethod subclass, including
CPPConstructor, whose executor is ConstructorExecutor. That one does not
return a PyObject* at all: it returns the address of the newly
constructed C++ object, cast to PyObject* for CPPConstructor::Call() to
unpack. Decref'ing that address treats arbitrary C++ object memory as a
PyObject and corrupts the heap.

Ask the method whether its executor hands back a real PyObject* before
dropping a reference. The object is leaked instead, which is acceptable
on the error path of a call that is about to be reported as failed.

🤖 Done with the help of AI
@guitargeek

Copy link
Copy Markdown
Contributor Author

Thanks for the review! I removed all non-cppyy commits from this PR, so this part can already been merged.

The part about using -W error for Python tests is anyway still blocked by deprecation warnings in the SOFIE parsers, which will go away soon:

Once they are gone, I'll re-start the effort with the Python development module.

@guitargeek guitargeek changed the title [test] Use Python development module with warnings as errors for testing [CPyCppyy] Do not decref a constructor result on Windows Aug 9, 2026
@guitargeek
guitargeek merged commit 52866ae into root-project:master Aug 9, 2026
33 of 34 checks passed
@guitargeek
guitargeek deleted the python_dev branch August 9, 2026 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants