Crash in PyQt/Python3 app on exit

Thu Jan 26 15:31:57 GMT 2023

Using the current pre-release packages at...

https://www.riverbankcomputing.com/pypi/simple/

...I can't reproduce the problem. test.py crashes and test-cleanup.py 
doesn't (as you are expecting).

Phil

On 25/01/2023 22:56, Kevin Constantine wrote:
> 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
>>> 
>> 