[PyQt] sip: Accessing members results in incorrect/missing data

[Phil Thompson]

On 24/01/2020 11:03, MatthijsBurgh wrote:
> Phil Thompson-5 wrote
>> On 22/01/2020 21:50, MatthijsBurgh wrote:
>>> Phil Thompson-5 wrote
>>>> On 22/01/2020 13:57, MatthijsBurgh wrote:
>>>>> The first changeset already fixed the issue.
>>>>> 
>>>>> The newer changeset creates the situation as desribed in
>>>>> http://python.6.x6.nabble.com/sip-Accessing-members-results-in-incorrect-missing-data-tp5266521p5266740.html,
>>>>> it looks like the release function isn't called anymore. I thought
>>>>> Python GC
>>>>> was able to break up circular references, but it looks like it 
>>>>> isn't
>>>>> happening.
>>>>> 
>>>>> By running the following code:
>>>>> ```
>>>>> v = Vector(1,2,3)
>>>>> R = Rotation()
>>>>> F = Frame(R,v)
>>>>> v2 = F.p
>>>>> v3 = Frame(F).p
>>>>> v3 = Vector(2,3,4)
>>>>> ```
>>>>> The `release_Frame` is called(print shows up) after the second `v3`
>>>>> assignment when using your first changeset, but the print doesn't
>>>>> show
>>>>> up
>>>>> with your second changeset, today's one.
>>>>> 
>>>>> So to me it looks like this will cause a memory leak.
>>>> 
>>>> In my own test cases the cycle is broken. Have you run gc.collect()?
>>>> If
>>>> you print out the reference counts to they look correct?
>>>> 
>>>>> Side note: I would prefer the usage of a specific key for the 
>>>>> member
>>>>> reference to the containing class.
>>>>> I don't think using the same key for both references will cause a
>>>>> problem
>>>>> with overwriting. As a class can't contain a member of the same 
>>>>> type,
>>>>> only a
>>>>> member as a pointer to the same type. In which case, the reference
>>>>> isn't
>>>>> kept, if I understand the code correct.
>>>> 
>>>> There can be a problem (now fixed) if the type of the variable is
>>>> defined in a different module from the type of the container.
>>>> 
>>>> Phil
>>>> _______________________________________________
>>>> PyQt mailing list
>>> 
>>>> PyQt@
>>> 
>>>> https://www.riverbankcomputing.com/mailman/listinfo/pyqt
>>> 
>>> In your last changeset (but also the second one), the new frame keeps
>>> alive.
>>> Even after `v3` is assigned a new value. Therefore the unassigned
>>> Frame,
>>> from which only `p` was accessed, stays alive till the end of the
>>> Python
>>> session.
>>> 
>>> This happens because `Frame` has a ref to `p`, because of the caching
>>> of the
>>> python object. And `p` has a reference to `Frame`, to keep the class
>>> alive,
>>> to be able to access the member `p`.
>>> 
>>> I don't see an easy solution for this at the moment. Do you?
>> 
>> As I said, I don't seem to see the problem. The caching is needed as
>> explained in the comments. Did you try gc.collect()?
>> 
>> Phil
>> _______________________________________________
>> PyQt mailing list
> 
>> PyQt@
> 
>> https://www.riverbankcomputing.com/mailman/listinfo/pyqt
> 
> In a manual test. the GC did not collect the items directly, just at 
> the end
> of the session. But if run in a loop, the GC does it job and the memory
> stays about constant.

Yes, the GC isn't invoked as soon as a reference count reaches 0.

> I have found another problem(incl. fix). While running the following 
> code:
> ```
> import PyKDL as kdl
> import sip
> 
> F = kdl.Frame(
>     kdl.Rotation.Quaternion(0, 1, 0, 0),
>     kdl.Vector(1, 2, 3))
> 
> while True:
>     v = F.p
> ```
> 
> This will cause a memory leak as shown in the image below.
> <http://python.6.x6.nabble.com/file/t383425/mem_leak.png>
> 
> The problem is caused in `sip_api_get_reference` in `siplib.c`.
> PyInt_FromLong(2.7) and PyLong_FromLong(3) return a new reference, see
> https://docs.python.org/2.7/c-api/int.html#c.PyInt_FromLong. Therefore 
> a
> `Py_DECREF(key_obj);` needs to be added before returning the object.
> 
> The first part of the memory graph show the test with the fix, the peek 
> is
> the test without the fix.

Applied, thanks.

> After implementing this fix, could you also release the newest 4.19.X
> version to ubuntu 16.04 & 18.04(and maybe some other distributions as 
> well)?

I will make a new release when Qt v5.14.1 is released. I have no control 
over what versions are used by Linux distros.

Phil