<div dir="ltr"><div class="gmail_default" style="font-family:courier new,monospace">Thanks Phil-  </div><div class="gmail_default" style="font-family:courier new,monospace">I'll give it a whirl and report back.  </div><div class="gmail_default" style="font-family:courier new,monospace"><br></div><div class="gmail_default" style="font-family:courier new,monospace">-kevin</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jan 10, 2023 at 12:13 PM 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">Sorry, but the first thing to do is upgrade to current versions of 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>
> and<br>
> here<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>
> 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>
</blockquote></div>