<div dir="ltr"><div class="gmail_default" style="font-family:courier new,monospace">If test.py crashes, that's reproducing the problem.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 26, 2023 at 10:31 AM Phil Thompson <<a href="mailto:phil@riverbankcomputing.com">phil@riverbankcomputing.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Using the current pre-release packages at...<br>
<br>
<a href="https://www.riverbankcomputing.com/pypi/simple/" rel="noreferrer" target="_blank">https://www.riverbankcomputing.com/pypi/simple/</a><br>
<br>
...I can't reproduce the problem. test.py crashes and test-cleanup.py <br>
doesn't (as you are expecting).<br>
<br>
Phil<br>
<br>
On 25/01/2023 22:56, Kevin Constantine wrote:<br>
> Hey Phil-<br>
> <br>
> I have an environment set up with:<br>
> Sip: 6.7.5<br>
> PyQt5: 5.15.7<br>
> Qt: 5.15.2<br>
> <br>
> I'm still seeing the crash.<br>
> <br>
> I've updated the reproducer repository with the changes to support the<br>
> newer version of Sip: <a href="https://github.com/kevinconstantine/pyqt-crash" rel="noreferrer" target="_blank">https://github.com/kevinconstantine/pyqt-crash</a><br>
> <br>
> -kevin<br>
> <br>
> On Tue, Jan 10, 2023 at 12:44 PM Kevin Constantine <<br>
> <a href="mailto:kevin.constantine@disneyanimation.com" target="_blank">kevin.constantine@disneyanimation.com</a>> wrote:<br>
> <br>
>> Thanks Phil-<br>
>> I'll give it a whirl and report back.<br>
>> <br>
>> -kevin<br>
>> <br>
>> On Tue, Jan 10, 2023 at 12:13 PM Phil Thompson <<br>
>> <a href="mailto:phil@riverbankcomputing.com" target="_blank">phil@riverbankcomputing.com</a>> wrote:<br>
>> <br>
>>> Sorry, but the first thing to do is upgrade to current versions of <br>
>>> SIP<br>
>>> and PyQt5.<br>
>>> <br>
>>> Phil<br>
>>> <br>
>>> On 09/01/2023 19:28, Kevin Constantine wrote:<br>
>>> > Hello!<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > We have a C++/Qt application that provides both a C++ api and a SIP<br>
>>> > wrapped<br>
>>> > Python api.  When converting everything from Python 2.7 to Python 3, we<br>
>>> > began to see crashes when python applications using our api<br>
>>> > exited/quit.  I<br>
>>> > have been tracking this crash down, and I think I have a decent<br>
>>> > understanding of what's causing it, which I'll attempt to explain<br>
>>> > below.<br>
>>> > However, I don't feel like I have a good understanding of why the code<br>
>>> > path<br>
>>> > that leads to the crash does what it does.  In other words, I'm not<br>
>>> > sure if<br>
>>> > it's expected behavior, or a bug.  That's what I'm hoping you might be<br>
>>> > able<br>
>>> > to assist with.<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > I have a simple-ish reproducer:<br>
>>> ><br>
>>> > <a href="https://github.com/kevinconstantine/pyqt-crash" rel="noreferrer" target="_blank">https://github.com/kevinconstantine/pyqt-crash</a><br>
>>> ><br>
>>> > that demonstrates the problem if needed. If it's a bug, I have a patch<br>
>>> > that<br>
>>> > prevents the crash, but given my current understanding of what's going<br>
>>> > on,<br>
>>> > it may not be the right direction.<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > Versions<br>
>>> ><br>
>>> > Python: 3.7.7<br>
>>> ><br>
>>> > Qt: 5.15.2<br>
>>> ><br>
>>> > PyQt: 5.15.4 (problem likely exists in 6.4.0 as well given code<br>
>>> > similarities)<br>
>>> ><br>
>>> > sip: 4.19.30<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > Summary<br>
>>> ><br>
>>> > It looks like in Python3, PyQt has registered a call-back when the<br>
>>> > application exits (cleanup_on_exit()).  The call chain eventually<br>
>>> > results in<br>
>>> > sip_api_visit_wrapper() looping over SIP objects and calling PyQt's<br>
>>> > cleanup_qobject().  cleanup_qobject() checks if the object is owned by<br>
>>> > Python and checks if the object is a QObject.  If the object is owned<br>
>>> > by<br>
>>> > C++ or not a QObject, it returns early.  Once we've established that<br>
>>> > the<br>
>>> > object is owned by Python, it looks like it transfers ownership of the<br>
>>> > Python sipWrapper object to C++, and then calls delete on the address<br>
>>> > of<br>
>>> > the sipWrapper object.<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > This delete causes the C++ destructors to get called, however, the SIP<br>
>>> > destructor is never called.  I would expect that a python owned object<br>
>>> > would have that SIP destructor called.  We are relying on doing some<br>
>>> > cleanup inside of the SIP destructor, and because that doesn't get<br>
>>> > executed, the application crashes.<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > More detail on what we're doing<br>
>>> ><br>
>>> > For years, we've leveraged a couple of examples (here<br>
>>> > <<a href="https://riverbankcomputing.com/pipermail/pyqt/2005-March/010031.html" rel="noreferrer" target="_blank">https://riverbankcomputing.com/pipermail/pyqt/2005-March/010031.html</a>>,<br>
>>> <br>
>>> > and<br>
>>> > here<br>
>>> > <<br>
>>> <a href="https://www.riverbankcomputing.com/pipermail/pyqt/2017-September/039548.html" rel="noreferrer" target="_blank">https://www.riverbankcomputing.com/pipermail/pyqt/2017-September/039548.html</a><br>
>>> >)<br>
>>> > to handle tracking shared pointers on the C++ side of things as they<br>
>>> > got<br>
>>> > created through Python.  We have a hand-coded SIP destructor that gets<br>
>>> > called from Python that removes the shared pointer from a map so that<br>
>>> > it<br>
>>> > too gets destructed properly.  This all worked great until we tried to<br>
>>> > migrate to Python 3 (cue the ominous music).<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > What's happening (at least my understanding of what's happening)<br>
>>> ><br>
>>> > In Python3, PyQt has registered cleanup_on_exit() with<br>
>>> > sipRegisterExitNotifier()<br>
>>> > in qpy/QtCore/qpycore_init.cpp.  So when the application is exiting,<br>
>>> > cleanup_on_exit() is called and the call-stack looks like:<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > PyQt: cleanup_on_exit()<br>
>>> ><br>
>>> >   PyQt: pyqt5_cleanup_qobjects()<br>
>>> ><br>
>>> >     SIP: sip_api_visit_wrappers() // Loop over sip objects<br>
>>> ><br>
>>> >       PyQt: cleanup_qobject()<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > Within qpy/QtCore/qpycore_public_api.cpp:cleanup_qobject(), several<br>
>>> > checks<br>
>>> > are made that cause the function to return early without doing<br>
>>> > anything:<br>
>>> ><br>
>>> > 1. Anything not owned by Python returns early<br>
>>> ><br>
>>> > 2. Non QObjects return early<br>
>>> ><br>
>>> > 3. Running threads return early<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > But if an object passes all of these checks, the code calls<br>
>>> > sipTransferTo()<br>
>>> > to transfer ownership of the sipWrapper object to C++, and then calls<br>
>>> > delete on the address of the sipWrapper object.<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > This delete causes the C++ destructors to get called, but most<br>
>>> > importantly,<br>
>>> > the SIP destructor that we're relying on, is never called.  As<br>
>>> > mentioned<br>
>>> > earlier, we are relying on the SIP destructor to clean up the global<br>
>>> > shared_ptr storage when the Python object is destroyed.  My expectation<br>
>>> > is<br>
>>> > that a Python owned object would have that SIP destructor called just<br>
>>> > like<br>
>>> > the SIP constructor is called when the object is created.<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > I added debugging output to SIP, PyQt, and my codebase in an effort to<br>
>>> > understand what was happening.  I'm hesitant to include that in this<br>
>>> > initial email as it gets kind of dense trying to explain what's going<br>
>>> > on,<br>
>>> > but I'm happy to provide it if I can interest anyone in going down the<br>
>>> > rabbit hole with me on this one.<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > I also have a small patch that "fixes" my crash, but I'm not sure that<br>
>>> > I<br>
>>> > completely have the right understanding of all of these pieces and that<br>
>>> > this doesn’t cause an issue elsewhere.<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > --- PyQt5-5.15.4.vanilla/qpy/QtCore/qpycore_public_api.cpp 2021-03-05<br>
>>> > 01:57:14.957003000 -0800<br>
>>> ><br>
>>> > +++ PyQt5-5.15.4.kcc/qpy/QtCore/qpycore_public_api.cpp 2022-12-15<br>
>>> > 08:40:06.644173000 -0800<br>
>>> ><br>
>>> > @@ -60,6 +60,13 @@<br>
>>> ><br>
>>> >              return;<br>
>>> ><br>
>>> >      }<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > +    // Try to destroy the object if it still has a reference<br>
>>> ><br>
>>> > +    // Goal is to get the SIP dtor to fire.<br>
>>> ><br>
>>> > +    if (Py_REFCNT((PyObject *)sw)) {<br>
>>> ><br>
>>> > +        sipInstanceDestroyed(sw);<br>
>>> ><br>
>>> > +        return;<br>
>>> ><br>
>>> > +    }<br>
>>> ><br>
>>> > +<br>
>>> ><br>
>>> >      sipTransferTo((PyObject *)sw, SIP_NULLPTR);<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> >      Py_BEGIN_ALLOW_THREADS<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > By calling sipInstanceDestroyed(), the SIP destructor fires, and<br>
>>> > subsequently the C++ dtors get executed as well.  Everything appears to<br>
>>> > get<br>
>>> > cleaned up in the proper order, and we avoid the crash.  I have not<br>
>>> > reproduced the issue in PyQt6, but there are no differences in<br>
>>> > cleanup_qobject() between the two versions, so I'm assuming the<br>
>>> > behavior is<br>
>>> > similar.<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > Thanks so much for any help you can provide in fixing this, or helping<br>
>>> > me<br>
>>> > understand better what's going on.<br>
>>> ><br>
>>> ><br>
>>> ><br>
>>> > -kevin<br>
>>> <br>
>> <br>
</blockquote></div>