Crash in PyQt/Python3 app on exit

[Kevin Constantine]

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/20230110/37f4e47f/attachment.htm>