[PyQt] [BUG] Return value of overriden QSGMaterial::createShader() impropely destroyed before use

[Phil Thompson]

On 18/06/2015 7:18 pm, Vladimir Rutsky wrote:
> On Thu, Jun 18, 2015 at 8:27 PM, Phil Thompson
> <phil at riverbankcomputing.com> wrote:
>> On 16/06/2015 3:30 pm, Vladimir Rutsky wrote:
>>> 
>>> On Mon, Jun 15, 2015 at 6:33 PM, Vladimir Rutsky
>>> <rutsky.vladimir at gmail.com> wrote:
>>>> 
>>>> Hello, Phil,
>>>> 
>>>> I made an example that reproduces this bug:
>>>> https://gist.github.com/rutsky/d6332e3354972997e938
>>>> 
>>>> Plus it shows another bug: application hangs on exit on GNU/Linux.
>>>> 
>>>> If I attach to it with GDB I can see, that application is waiting 
>>>> for
>>>> some condition variable in QSGRenderLoop::cleanup().
>>>> 
>>>>     0  pthread_cond_wait@@GLIBC_2.3.2 () at
>>>> ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
>>>>     1  0x00007f719d0f9cc3 in QWaitCondition::wait(QMutex*, unsigned
>>>> long) () from /home/bob/Qt/5.4/gcc_64/lib/libQt5Core.so.5
>>>>     2  0x00007f719e6f0acb in ?? () from
>>>> /home/bob/Qt/5.4/gcc_64/lib/libQt5Quick.so.5
>>>>     3  0x00007f719e6f12a0 in ?? () from
>>>> /home/bob/Qt/5.4/gcc_64/lib/libQt5Quick.so.5
>>>>     4  0x00007f719e6e7268 in QSGRenderLoop::cleanup() () from
>>>> /home/bob/Qt/5.4/gcc_64/lib/libQt5Quick.so.5
>>>>     5  0x00007f719d32e049 in qt_call_post_routines() () from
>>>> /home/bob/Qt/5.4/gcc_64/lib/libQt5Core.so.5
>>>>     6  0x00007f7197dc6a66 in QApplication::~QApplication() () from
>>>> /home/bob/Qt/5.4/gcc_64/lib/libQt5Widgets.so.5
>>>>     7  0x00007f7198892768 in sipQApplication::~sipQApplication
>>>> (this=0x259ba90, __in_chrg=<optimized out>) at
>>>> sipQtWidgetspart0.cpp:301912
>>>>     8  0x00007f7198892798 in sipQApplication::~sipQApplication
>>>> (this=0x259ba90, __in_chrg=<optimized out>) at
>>>> sipQtWidgetspart0.cpp:301915
>>>>     9  0x00007f7198895f31 in release_QApplication 
>>>> (sipCppV=0x259ba90)
>>>> at sipQtWidgetspart0.cpp:303524
>>>>     10 0x00007f7198895fca in dealloc_QApplication
>>>> (sipSelf=0x7f7197809168) at sipQtWidgetspart0.cpp:303538
>>>>     11 0x00007f7199d5b346 in ?? () from
>>>> /home/bob/work/proplan/env/lib/python3.4/site-packages/sip.so
>>>>     12 0x00007f7199d5c5e9 in ?? () from
>>>> /home/bob/work/proplan/env/lib/python3.4/site-packages/sip.so
>>>>     13 0x0000000000515c82 in ?? ()
>>>>     14 0x00000000004fb938 in ?? ()
>>>>     15 0x0000000000511820 in ?? ()
>>>>     16 0x00000000005117fe in ?? ()
>>>>     17 0x00000000005ae979 in ?? ()
>>>>     18 0x00000000005972bd in PyErr_PrintEx ()
>>>>     19 0x0000000000596331 in PyRun_SimpleFileExFlags ()
>>>>     20 0x00000000004cb93a in Py_Main ()
>>>>     21 0x00000000004cad1f in main ()
>>>> 
>>>> (This issue is not reproduces in Windows, since Qt 5.4 still uses 
>>>> single
>>>> threaded rendering there.)
>>>> 
>>>> 
>>>> Regards,
>>>> 
>>>> Vladimir Rutsky
>>> 
>>> 
>>> I have updated the example:
>>> https://gist.github.com/rutsky/d6332e3354972997e938
>>> 
>>> As I see, when QQuickView is being destroyed it waits until render 
>>> thread
>>> clean ups all resources, which causes custom created QSGGeometryNode
>>> to be destroyed,
>>> which asks to lock GIL, which is still locked by main thread, so it's
>>> deadlock.
>> 
>> 
>> Both problems should be fixed in tonight's snapshot.
>> 
>> I'd be interested to know how you think the ownership of the QSGNode
>> returned from updatePaintNode() should be handled. Where in the docs 
>> does it
>> say that Qt handles the lifetime of the node?
>> 
>> Thanks,
>> Phil
>> 
> 
> I made conclusion about ownership of QSGNode from example in
> QQuickItem::updatePaintNode documentation:
> http://doc.qt.io/qt-5/qquickitem.html#updatePaintNode
> 
> QSGNode *MyItem::updatePaintNode(QSGNode *node, UpdatePaintNodeData *)
> {
>     QSGSimpleRectNode *n = static_cast<QSGSimpleRectNode *>(node);
>     if (!n) {
>         n = new QSGSimpleRectNode();
>         n->setColor(Qt::red);
>     }
>     n->setRect(boundingRect());
>     return n;
> }
> 
> QSGSimpleRectNode is created on heap and wasn't stored anywere, just
> returned as QSGNode * pointer.
> 
> Now I see, that documentation has following information about QSGNode
> ownership: 
> http://doc.qt.io/qt-5/qquickitem.html#graphics-resource-handling
> 
>> A QSGNode returned from QQuickItem::updatePaintNode() is automatically 
>> deleted on the right thread at the right time.
>> Trees of QSGNode instances are managed through the use of 
>> QSGNode::OwnedByParent, which is set by default.
>> So, for the majority of custom scene graph items, no extra work will 
>> be required.
> 
> 
> So I was not completely really correct about ownership of QSGNode:
> it's defined by QSGNode::OwnedByParent flag which is set by default.

The ownership of the returned QSGNode should now be automatically 
handled correctly in tonight's snapshot.

Thanks,
Phil