[PyQt] [BUG] Issues with overriding QSGMaterialShader::attributeNames()

[Vladimir Rutsky]

On Sat, Jun 20, 2015 at 1:31 AM, Phil Thompson
<phil at riverbankcomputing.com> wrote:
> On 18/06/2015 7:49 pm, Vladimir Rutsky wrote:
>>
>> On Thu, Jun 18, 2015 at 8:43 PM, Phil Thompson
>> <phil at riverbankcomputing.com> wrote:
>>>
>>> On 02/06/2015 11:47 am, Vladimir Rutsky wrote:
>>>>
>>>>
>>>> Hello,
>>>>
>>>> 1. Overrided in Python QSGMaterialShader::attributeNames() always
>>>> returns NULL to Qt C++ code.
>>>>
>>>> In PyQt-gpl-5.4.1/sip/QtQuick/qsgmaterial.sip code for
>>>> attributeNames() method stores result in variable "result" and never
>>>> touches "sipRes" variable, which is returned at the end of function.
>>>>
>>>> Attached patch fixes this issue by assigning "sipRes" to "result" (I'm
>>>> not sure is this intented behavior in case of errors).
>>>>
>>>> 2. Calling overriden in Python QSGMaterialShader::attributeNames()
>>>> from Qt C++ code leads to memory leaks.
>>>>
>>>> Each call to attributeNames() creates copy of strings
>>>> (PyQt-gpl-5.4.1/sip/QtQuick/qsgmaterial.sip):
>>>>
>>>>                 // Note that the converted value will leak.  This
>>>> shouldn't matter
>>>>                 // because the data should be static anyway.
>>>>                 result = new char *[PyList_GET_SIZE(names) + 1];
>>>>                 result[PyList_GET_SIZE(names)] = 0;
>>>>
>>>> And attributeNames() is called e.g. in each
>>>> Renderer::renderUnmergedBatch(), which can lead to unlimited memory
>>>> leaks.
>>>
>>>
>>>
>>> (1) is fixed but is it possible you could produce a short script that
>>> demonstrates (2). I know it will leak but (as I'm not an OpenGL
>>> programmer)
>>> it would be really useful to see how this method is normally called.
>>>
>>> Thanks,
>>> Phil
>>
>>
>>
>> I made example where attributeNames() are called approximately every
>> 10 ms: https://gist.github.com/rutsky/380932afec27ff0f108e
>
>
> Should be fixed in tonight's snapshot.
>
> On OSX (at least) your script still leaks because the shader fails to
> compile because of the long name.
>
> Thanks,
> Phil

Thanks, Phil, memory leak in attributeNames() is not reproduced in the
latest nightly snapshot (d5b00e76067f).
There are some other leaks according to Valgrind, but hopefully they
are not as massive, as can be leak in attributeNames().

Regards,

Vladimir Rutsky