[prev in list] [next in list] [prev in thread] [next in thread] 

List:       pykde
Subject:    Re: [PyQt] sip: Accessing members results in incorrect/missing data
From:       Phil Thompson <phil () riverbankcomputing ! com>
Date:       2020-01-24 11:49:10
Message-ID: c06502a4cae2fa30efce54389e6d1b57 () riverbankcomputing ! com
[Download RAW message or body]

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
_______________________________________________
PyQt mailing list    PyQt@riverbankcomputing.com
https://www.riverbankcomputing.com/mailman/listinfo/pyqt


[prev in list] [next in list] [prev in thread] [next in thread]