Crash in PyQt/Python3 app on exit
Kevin Constantine
kevin.constantine at disneyanimation.com
Tue Jan 31 23:30:40 GMT 2023
Thanks Phil! I started to mock up something similar to what PyQt is doing
where it registers a handler with atexit(). The issue that I'm running
into currently is that PyQt's handler is getting called before mine, and
PyQt is destroying these objects, so in my handler where I'm also trying to
use
sipVisitWrappers(handler_func, QCoreApplication::instance());
to iterate over the SIP objects so that I can try to call their
destructors, or delete the underlying data, there's no longer anything to
iterate over.
Going back to my original question: when PyQt is cleaning things up, it's
taking a Python owned object, and passing the ownership over to C++ and
then destroying it. That causes the C++ destructor to get called, but not
the SIP destructor. That transfer of ownership feels incorrect, but again,
this isn't my domain, so I don't really know for sure.
-kevin
On Thu, Jan 26, 2023 at 11:54 AM Phil Thompson <phil at riverbankcomputing.com>
wrote:
> SIP deliberately suppresses the invocation of Python reimplementations
> of C++ virtuals once the interpreter starts to exit. It registers a
> handler with atexit() to do this.
>
> I'd suggest you register your own tidy-up handler from within your
> bindings. This is guaranteed to be called before SIP's handler.
>
> Phil
>
> On 26/01/2023 15:34, Kevin Constantine wrote:
> > If test.py crashes, that's reproducing the problem.
> >
> > On Thu, Jan 26, 2023 at 10:31 AM Phil Thompson
> > <phil at riverbankcomputing.com>
> > wrote:
> >
> >> 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
> >> >>>
> >> >>
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://www.riverbankcomputing.com/pipermail/pyqt/attachments/20230131/446665d4/attachment-0001.htm>
More information about the PyQt
mailing list