Crash in PyQt/Python3 app on exit

[Kevin Constantine]

Hello!



We have a C++/Qt application that provides both a C++ api and a SIP wrapped
Python api.  When converting everything from Python 2.7 to Python 3, we
began to see crashes when python applications using our api exited/quit.  I
have been tracking this crash down, and I think I have a decent
understanding of what's causing it, which I'll attempt to explain below.
However, I don't feel like I have a good understanding of why the code path
that leads to the crash does what it does.  In other words, I'm not sure if
it's expected behavior, or a bug.  That's what I'm hoping you might be able
to assist with.



I have a simple-ish reproducer:

https://github.com/kevinconstantine/pyqt-crash

that demonstrates the problem if needed. If it's a bug, I have a patch that
prevents the crash, but given my current understanding of what's going on,
it may not be the right direction.



Versions

Python: 3.7.7

Qt: 5.15.2

PyQt: 5.15.4 (problem likely exists in 6.4.0 as well given code
similarities)

sip: 4.19.30



Summary

It looks like in Python3, PyQt has registered a call-back when the
application exits (cleanup_on_exit()).  The call chain eventually results in
sip_api_visit_wrapper() looping over SIP objects and calling PyQt's
cleanup_qobject().  cleanup_qobject() checks if the object is owned by
Python and checks if the object is a QObject.  If the object is owned by
C++ or not a QObject, it returns early.  Once we've established that the
object is owned by Python, it looks like it transfers ownership of the
Python sipWrapper object to C++, and then calls delete on the address of
the sipWrapper object.



This delete causes the C++ destructors to get called, however, the SIP
destructor is never called.  I would expect that a python owned object
would have that SIP destructor called.  We are relying on doing some
cleanup inside of the SIP destructor, and because that doesn't get
executed, the application crashes.





More detail on what we're doing

For years, we've leveraged a couple of examples (here
<https://riverbankcomputing.com/pipermail/pyqt/2005-March/010031.html>, and
here
<https://www.riverbankcomputing.com/pipermail/pyqt/2017-September/039548.html>)
to handle tracking shared pointers on the C++ side of things as they got
created through Python.  We have a hand-coded SIP destructor that gets
called from Python that removes the shared pointer from a map so that it
too gets destructed properly.  This all worked great until we tried to
migrate to Python 3 (cue the ominous music).



What's happening (at least my understanding of what's happening)

In Python3, PyQt has registered cleanup_on_exit() with
sipRegisterExitNotifier()
in qpy/QtCore/qpycore_init.cpp.  So when the application is exiting,
cleanup_on_exit() is called and the call-stack looks like:



PyQt: cleanup_on_exit()

  PyQt: pyqt5_cleanup_qobjects()

    SIP: sip_api_visit_wrappers() // Loop over sip objects

      PyQt: cleanup_qobject()



Within qpy/QtCore/qpycore_public_api.cpp:cleanup_qobject(), several checks
are made that cause the function to return early without doing anything:

1. Anything not owned by Python returns early

2. Non QObjects return early

3. Running threads return early



But if an object passes all of these checks, the code calls sipTransferTo()
to transfer ownership of the sipWrapper object to C++, and then calls
delete on the address of the sipWrapper object.



This delete causes the C++ destructors to get called, but most importantly,
the SIP destructor that we're relying on, is never called.  As mentioned
earlier, we are relying on the SIP destructor to clean up the global
shared_ptr storage when the Python object is destroyed.  My expectation is
that a Python owned object would have that SIP destructor called just like
the SIP constructor is called when the object is created.



I added debugging output to SIP, PyQt, and my codebase in an effort to
understand what was happening.  I'm hesitant to include that in this
initial email as it gets kind of dense trying to explain what's going on,
but I'm happy to provide it if I can interest anyone in going down the
rabbit hole with me on this one.



I also have a small patch that "fixes" my crash, but I'm not sure that I
completely have the right understanding of all of these pieces and that
this doesn’t cause an issue elsewhere.



--- PyQt5-5.15.4.vanilla/qpy/QtCore/qpycore_public_api.cpp 2021-03-05
01:57:14.957003000 -0800

+++ PyQt5-5.15.4.kcc/qpy/QtCore/qpycore_public_api.cpp 2022-12-15
08:40:06.644173000 -0800

@@ -60,6 +60,13 @@

             return;

     }



+    // Try to destroy the object if it still has a reference

+    // Goal is to get the SIP dtor to fire.

+    if (Py_REFCNT((PyObject *)sw)) {

+        sipInstanceDestroyed(sw);

+        return;

+    }

+

     sipTransferTo((PyObject *)sw, SIP_NULLPTR);



     Py_BEGIN_ALLOW_THREADS



By calling sipInstanceDestroyed(), the SIP destructor fires, and
subsequently the C++ dtors get executed as well.  Everything appears to get
cleaned up in the proper order, and we avoid the crash.  I have not
reproduced the issue in PyQt6, but there are no differences in
cleanup_qobject() between the two versions, so I'm assuming the behavior is
similar.



Thanks so much for any help you can provide in fixing this, or helping me
understand better what's going on.



-kevin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://www.riverbankcomputing.com/pipermail/pyqt/attachments/20230109/6340fd3b/attachment-0001.htm>