Crash in PyQt/Python3 app on exit

[Kevin Constantine]

Hey Phil-

I have an environment set up with:
Sip: 6.7.5
PyQt5: 5.15.7
Qt: 5.15.2

I'm still seeing the crash.

I've updated the reproducer repository with the changes to support the
newer version of Sip: https://github.com/kevinconstantine/pyqt-crash

-kevin

On Tue, Jan 10, 2023 at 12:44 PM Kevin Constantine <
kevin.constantine at disneyanimation.com> wrote:

> Thanks Phil-
> I'll give it a whirl and report back.
>
> -kevin
>
> On Tue, Jan 10, 2023 at 12:13 PM Phil Thompson <
> phil at riverbankcomputing.com> wrote:
>
>> Sorry, but the first thing to do is upgrade to current versions of SIP
>> and PyQt5.
>>
>> Phil
>>
>> On 09/01/2023 19:28, Kevin Constantine wrote:
>> > 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/20230125/be0d7b2b/attachment-0001.htm>