sip: Accessing members results in incorrect/missing data

Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

sip: Accessing members results in incorrect/missing data

MatthijsBurgh
Hey All,

I think my problem is related to http://python.6.x6.nabble.com/sip-Member-objects-and-GC-td4503261.html, but this thread doesn't provide me any solution.

I have been using the following library for a long time: https://github.com/orocos/orocos_kinematics_dynamics

I have been using ubuntu 16.04 with sip 4.15.5 for a long time. Now I am moving to ubuntu 18.04 and sip 4.15.5 isn't working anymore.

Some versions of SIP will cause a segmentation fault on import of PyKDL, others will cause the following issue.

By accessing a member directly after calling a constructor or other operations which will return a new instance, the data of the member is incorrect.

By executing the following code:
```
# Part of an unittest.Testcase
v=Vector(3,4,5)
v1=Vector(4,-2,1)
w=Wrench(Vector(7,-1,3),Vector(2,-3,3))
t=Twist(Vector(6,3,5),Vector(4,-2,7))
R = Rotation.EulerZYX(radians(10),radians(20),radians(-10))
F = Frame(R, v1)
F2 = Frame(F)
v2 = Frame(F).p
self.assertEqual(F,F2)
self.assertEqual(F.p,Frame(F).p)
```
This last line will result in the following error:
```
ts/framestest.py", line 150, in testFrame
    self.assertEqual(F.p,Frame(F).p)
AssertionError: [           4,          -2,           1] != [           0,          -2,           1]
```
I have tried to locate the changes which caused the problem. I did this on ubuntu 16.04 as there 4.15.5 does work. I used the master branch of the KDL libary. I have started from 4.15.5 up to newer version till the error started to show up. I found that 4.16.8 is the first version which breaks the bindings. Especially changeset 11a92ebd4840, https://www.riverbankcomputing.com/hg/sip/rev/11a92ebd4840.

I hope you could help.

Kind regards,
Matthijs van der Burgh


_______________________________________________
PyQt mailing list    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

Phil Thompson-5
On 12/01/2020 23:11, Matthijs van der Burgh wrote:

> Hey All,
>
> I think my problem is related to
> http://python.6.x6.nabble.com/sip-Member-objects-and-GC-td4503261.html,
> but this thread doesn't provide me any solution.
>
> I have been using the following library for a long time:
> https://github.com/orocos/orocos_kinematics_dynamics
>
> I have been using ubuntu 16.04 with sip 4.15.5 for a long time. Now I
> am moving to ubuntu 18.04 and sip 4.15.5 isn't working anymore.
>
> Some versions of SIP will cause a segmentation fault on import of
> PyKDL, others will cause the following issue.
>
> By accessing a member directly after calling a constructor or other
> operations which will return a new instance, the data of the member is
> incorrect.
>
> By executing the following code:
> ```
> # Part of an unittest.Testcase
> v=Vector(3,4,5)
> v1=Vector(4,-2,1)
> w=Wrench(Vector(7,-1,3),Vector(2,-3,3))
> t=Twist(Vector(6,3,5),Vector(4,-2,7))
> R = Rotation.EulerZYX(radians(10),radians(20),radians(-10))
> F = Frame(R, v1)
> F2 = Frame(F)
> v2 = Frame(F).p
> self.assertEqual(F,F2)
> self.assertEqual(F.p,Frame(F).p)
> ```
> This last line will result in the following error:
> ```
> ts/framestest.py", line 150, in testFrame
>     self.assertEqual(F.p,Frame(F).p)
> AssertionError: [           4,          -2,           1] != [
>  0,          -2,           1]
> ```
> I have tried to locate the changes which caused the problem. I did
> this on ubuntu 16.04 as there 4.15.5 does work. I used the master
> branch of the KDL libary. I have started from 4.15.5 up to newer
> version till the error started to show up. I found that 4.16.8 is the
> first version which breaks the bindings. Especially changeset
> 11a92ebd4840,
> https://www.riverbankcomputing.com/hg/sip/rev/11a92ebd4840.
>
> I hope you could help.

You need to talk to the authors of the bindings.

Phil
_______________________________________________
PyQt mailing list    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

MatthijsBurgh
Also the author of the bindings, smits, has no clue about how to fix this
issue. The repo contains already 3 issues related to this problem with the
earliest dating back to 2014.

https://github.com/orocos/orocos_kinematics_dynamics/issues/41
<https://github.com/orocos/orocos_kinematics_dynamics/issues/41>  
https://github.com/orocos/orocos_kinematics_dynamics/issues/129
<https://github.com/orocos/orocos_kinematics_dynamics/issues/129>  
https://github.com/orocos/orocos_kinematics_dynamics/issues/157
<https://github.com/orocos/orocos_kinematics_dynamics/issues/157>  

As could be concluded from the issues, the cpp object seems to be correct,
but the bindings behave incorrect.

Would be really appreciated if you could investigate it a little bit. I
could provide you all lines of code which are important.

Matthijs



--
Sent from: http://python.6.x6.nabble.com/PyQt-f1792048.html
_______________________________________________
PyQt mailing list    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

Phil Thompson-5
On 13/01/2020 10:46, MatthijsBurgh wrote:

> Also the author of the bindings, smits, has no clue about how to fix
> this
> issue. The repo contains already 3 issues related to this problem with
> the
> earliest dating back to 2014.
>
> https://github.com/orocos/orocos_kinematics_dynamics/issues/41
> <https://github.com/orocos/orocos_kinematics_dynamics/issues/41>
> https://github.com/orocos/orocos_kinematics_dynamics/issues/129
> <https://github.com/orocos/orocos_kinematics_dynamics/issues/129>
> https://github.com/orocos/orocos_kinematics_dynamics/issues/157
> <https://github.com/orocos/orocos_kinematics_dynamics/issues/157>
>
> As could be concluded from the issues, the cpp object seems to be
> correct,
> but the bindings behave incorrect.
>
> Would be really appreciated if you could investigate it a little bit. I
> could provide you all lines of code which are important.

Sorry, if the author can't be bothered to investigate to determine if
the problem is with their own code or not (rather than my 5 year old
code) then I don't see why I should do it for them.

Phil
_______________________________________________
PyQt mailing list    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

MatthijsBurgh
Phil Thompson-5 wrote

> On 13/01/2020 10:46, MatthijsBurgh wrote:
>> Also the author of the bindings, smits, has no clue about how to fix
>> this
>> issue. The repo contains already 3 issues related to this problem with
>> the
>> earliest dating back to 2014.
>>
>> https://github.com/orocos/orocos_kinematics_dynamics/issues/41
>> &lt;https://github.com/orocos/orocos_kinematics_dynamics/issues/41&gt;
>> https://github.com/orocos/orocos_kinematics_dynamics/issues/129
>> &lt;https://github.com/orocos/orocos_kinematics_dynamics/issues/129&gt;
>> https://github.com/orocos/orocos_kinematics_dynamics/issues/157
>> &lt;https://github.com/orocos/orocos_kinematics_dynamics/issues/157&gt;
>>
>> As could be concluded from the issues, the cpp object seems to be
>> correct,
>> but the bindings behave incorrect.
>>
>> Would be really appreciated if you could investigate it a little bit. I
>> could provide you all lines of code which are important.
>
> Sorry, if the author can't be bothered to investigate to determine if
> the problem is with their own code or not (rather than my 5 year old
> code) then I don't see why I should do it for them.
>
> Phil
> _______________________________________________
> PyQt mailing list    

> PyQt@

> https://www.riverbankcomputing.com/mailman/listinfo/pyqt

I wouldn't say the author isn't bothered. He has contributed to issues
mentioned above. He is the creator of the oldest issue. I just took the
initiative myself.

I have just mentioned this thread in
https://github.com/orocos/orocos_kinematics_dynamics/issues/41. I am sure he
is willing to help as well.

About the 5 year old code, I still experience the issue with release
4.19.20. But I think this is still related to changes from the changeset
mentioned above.

Matthijs





--
Sent from: http://python.6.x6.nabble.com/PyQt-f1792048.html
_______________________________________________
PyQt mailing list    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

MatthijsBurgh
In reply to this post by Phil Thompson-5
I have done some more investigating.

As mentioned https://github.com/orocos/orocos_kinematics_dynamics/issues/41,
problems where occuring in with SIP 4.13.2.

Which is explainable as you fixed this issue in the commit:
https://www.riverbankcomputing.com/hg/sip/rev/0dd3cb4eff0e. This makes sure
the member variable contains a reference to the containing class. This
prevents the class from being garbage collected while there are still
references to the member variable.

You apply a fix to this in commit
https://www.riverbankcomputing.com/hg/sip/rev/fbb9cdbad791.

The commit, https://www.riverbankcomputing.com/hg/sip/rev/11a92ebd4840,
which I also mentioned in my first message, is the important one.

In this commit you introduce a kind of caching for member variables. The
python wrapper of the members of a class are stored in a dict in the python
wrapper of the class. To add the member to the cache, you use
"sipKeepReference(sipPySelf, %d, sipPy)". So the class contains a reference
to the member variable.

While previously "sipKeepReference(sipPy, -1, sipPySelf)", was used to store
the reference to the class in the dict of the member variable. Focus here on
the switch of "sipPy" and "sipPySelf". So by the last commit, 11a92ebd4840,
you added a new feature, but you broke another one.

I suggest to restore the feature which stores the reference to the class in
the member variable dict.

I did some testing in 4.19.20. I didn't experience a significant performance
increase with the caching with my Frame object and its member p, a vector
with 3 variables. (I tweaked the code,
https://www.riverbankcomputing.com/hg/sip/file/7c54145e55b6/sipgen/gencode.c#l4904,
to always get the caching.)
Also I added manually the "sipKeepReference(sipPy, -1, sipPySelf)" to the
generated cpp for the class reference. When both features were used at the
same time, the "release" function of the class was not called. (I added a
"printf" to it, which didn't show in that situation, but did if just of the
two was used.)

I hope you are still willing to help with this issue.

The library is open-source and mostly community-driven in the last few years
as the maintaining this library is not related to the maintainers daily job.
So the maintainer is not able to put in as much time to fix this problem as
I can.

Matthijs




--
Sent from: http://python.6.x6.nabble.com/PyQt-f1792048.html
_______________________________________________
PyQt mailing list    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

Phil Thompson-5
On 16/01/2020 20:52, MatthijsBurgh wrote:

> I have done some more investigating.
>
> As mentioned
> https://github.com/orocos/orocos_kinematics_dynamics/issues/41,
> problems where occuring in with SIP 4.13.2.
>
> Which is explainable as you fixed this issue in the commit:
> https://www.riverbankcomputing.com/hg/sip/rev/0dd3cb4eff0e. This makes
> sure
> the member variable contains a reference to the containing class. This
> prevents the class from being garbage collected while there are still
> references to the member variable.
>
> You apply a fix to this in commit
> https://www.riverbankcomputing.com/hg/sip/rev/fbb9cdbad791.
>
> The commit, https://www.riverbankcomputing.com/hg/sip/rev/11a92ebd4840,
> which I also mentioned in my first message, is the important one.
>
> In this commit you introduce a kind of caching for member variables.
> The
> python wrapper of the members of a class are stored in a dict in the
> python
> wrapper of the class. To add the member to the cache, you use
> "sipKeepReference(sipPySelf, %d, sipPy)". So the class contains a
> reference
> to the member variable.
>
> While previously "sipKeepReference(sipPy, -1, sipPySelf)", was used to
> store
> the reference to the class in the dict of the member variable. Focus
> here on
> the switch of "sipPy" and "sipPySelf". So by the last commit,
> 11a92ebd4840,
> you added a new feature, but you broke another one.
>
> I suggest to restore the feature which stores the reference to the
> class in
> the member variable dict.
>
> I did some testing in 4.19.20. I didn't experience a significant
> performance
> increase with the caching with my Frame object and its member p, a
> vector
> with 3 variables. (I tweaked the code,
> https://www.riverbankcomputing.com/hg/sip/file/7c54145e55b6/sipgen/gencode.c#l4904,
> to always get the caching.)
> Also I added manually the "sipKeepReference(sipPy, -1, sipPySelf)" to
> the
> generated cpp for the class reference. When both features were used at
> the
> same time, the "release" function of the class was not called. (I added
> a
> "printf" to it, which didn't show in that situation, but did if just of
> the
> two was used.)

Thanks for the analysis, that was very helpful.

This changeset is also a problem...

https://www.riverbankcomputing.com/hg/sip/rev/6ec87337d5e2

Can you try the latest version of the 4.19-maint branch, ie. including
this changeset...

https://www.riverbankcomputing.com/hg/sip/rev/137b9be794a1

...and let me know how you get on.

Phil
_______________________________________________
PyQt mailing list    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

Phil Thompson-5
On 21/01/2020 17:50, Phil Thompson wrote:

> On 16/01/2020 20:52, MatthijsBurgh wrote:
>> I have done some more investigating.
>>
>> As mentioned
>> https://github.com/orocos/orocos_kinematics_dynamics/issues/41,
>> problems where occuring in with SIP 4.13.2.
>>
>> Which is explainable as you fixed this issue in the commit:
>> https://www.riverbankcomputing.com/hg/sip/rev/0dd3cb4eff0e. This makes
>> sure
>> the member variable contains a reference to the containing class. This
>> prevents the class from being garbage collected while there are still
>> references to the member variable.
>>
>> You apply a fix to this in commit
>> https://www.riverbankcomputing.com/hg/sip/rev/fbb9cdbad791.
>>
>> The commit,
>> https://www.riverbankcomputing.com/hg/sip/rev/11a92ebd4840,
>> which I also mentioned in my first message, is the important one.
>>
>> In this commit you introduce a kind of caching for member variables.
>> The
>> python wrapper of the members of a class are stored in a dict in the
>> python
>> wrapper of the class. To add the member to the cache, you use
>> "sipKeepReference(sipPySelf, %d, sipPy)". So the class contains a
>> reference
>> to the member variable.
>>
>> While previously "sipKeepReference(sipPy, -1, sipPySelf)", was used to
>> store
>> the reference to the class in the dict of the member variable. Focus
>> here on
>> the switch of "sipPy" and "sipPySelf". So by the last commit,
>> 11a92ebd4840,
>> you added a new feature, but you broke another one.
>>
>> I suggest to restore the feature which stores the reference to the
>> class in
>> the member variable dict.
>>
>> I did some testing in 4.19.20. I didn't experience a significant
>> performance
>> increase with the caching with my Frame object and its member p, a
>> vector
>> with 3 variables. (I tweaked the code,
>> https://www.riverbankcomputing.com/hg/sip/file/7c54145e55b6/sipgen/gencode.c#l4904,
>> to always get the caching.)
>> Also I added manually the "sipKeepReference(sipPy, -1, sipPySelf)" to
>> the
>> generated cpp for the class reference. When both features were used at
>> the
>> same time, the "release" function of the class was not called. (I
>> added a
>> "printf" to it, which didn't show in that situation, but did if just
>> of the
>> two was used.)
>
> Thanks for the analysis, that was very helpful.
>
> This changeset is also a problem...
>
> https://www.riverbankcomputing.com/hg/sip/rev/6ec87337d5e2
>
> Can you try the latest version of the 4.19-maint branch, ie. including
> this changeset...
>
> https://www.riverbankcomputing.com/hg/sip/rev/137b9be794a1
>
> ...and let me know how you get on.

There is now a newer changeset...

https://www.riverbankcomputing.com/hg/sip/rev/6a057b2d8537

Phil
_______________________________________________
PyQt mailing list    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

MatthijsBurgh
Phil Thompson-5 wrote

> On 21/01/2020 17:50, Phil Thompson wrote:
>> On 16/01/2020 20:52, MatthijsBurgh wrote:
>>> I have done some more investigating.
>>>
>>> As mentioned
>>> https://github.com/orocos/orocos_kinematics_dynamics/issues/41,
>>> problems where occuring in with SIP 4.13.2.
>>>
>>> Which is explainable as you fixed this issue in the commit:
>>> https://www.riverbankcomputing.com/hg/sip/rev/0dd3cb4eff0e. This makes
>>> sure
>>> the member variable contains a reference to the containing class. This
>>> prevents the class from being garbage collected while there are still
>>> references to the member variable.
>>>
>>> You apply a fix to this in commit
>>> https://www.riverbankcomputing.com/hg/sip/rev/fbb9cdbad791.
>>>
>>> The commit,
>>> https://www.riverbankcomputing.com/hg/sip/rev/11a92ebd4840,
>>> which I also mentioned in my first message, is the important one.
>>>
>>> In this commit you introduce a kind of caching for member variables.
>>> The
>>> python wrapper of the members of a class are stored in a dict in the
>>> python
>>> wrapper of the class. To add the member to the cache, you use
>>> "sipKeepReference(sipPySelf, %d, sipPy)". So the class contains a
>>> reference
>>> to the member variable.
>>>
>>> While previously "sipKeepReference(sipPy, -1, sipPySelf)", was used to
>>> store
>>> the reference to the class in the dict of the member variable. Focus
>>> here on
>>> the switch of "sipPy" and "sipPySelf". So by the last commit,
>>> 11a92ebd4840,
>>> you added a new feature, but you broke another one.
>>>
>>> I suggest to restore the feature which stores the reference to the
>>> class in
>>> the member variable dict.
>>>
>>> I did some testing in 4.19.20. I didn't experience a significant
>>> performance
>>> increase with the caching with my Frame object and its member p, a
>>> vector
>>> with 3 variables. (I tweaked the code,
>>> https://www.riverbankcomputing.com/hg/sip/file/7c54145e55b6/sipgen/gencode.c#l4904,
>>> to always get the caching.)
>>> Also I added manually the "sipKeepReference(sipPy, -1, sipPySelf)" to
>>> the
>>> generated cpp for the class reference. When both features were used at
>>> the
>>> same time, the "release" function of the class was not called. (I
>>> added a
>>> "printf" to it, which didn't show in that situation, but did if just
>>> of the
>>> two was used.)
>>
>> Thanks for the analysis, that was very helpful.
>>
>> This changeset is also a problem...
>>
>> https://www.riverbankcomputing.com/hg/sip/rev/6ec87337d5e2
>>
>> Can you try the latest version of the 4.19-maint branch, ie. including
>> this changeset...
>>
>> https://www.riverbankcomputing.com/hg/sip/rev/137b9be794a1
>>
>> ...and let me know how you get on.
>
> There is now a newer changeset...
>
> https://www.riverbankcomputing.com/hg/sip/rev/6a057b2d8537
>
> Phil
> _______________________________________________
> PyQt mailing list    

> PyQt@

> https://www.riverbankcomputing.com/mailman/listinfo/pyqt

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.

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.

Matthijs



--
Sent from: http://python.6.x6.nabble.com/PyQt-f1792048.html
_______________________________________________
PyQt mailing list    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

Phil Thompson-5
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    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

MatthijsBurgh
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?

Matthijs




--
Sent from: http://python.6.x6.nabble.com/PyQt-f1792048.html
_______________________________________________
PyQt mailing list    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

Phil Thompson-5
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    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

MatthijsBurgh
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.

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.

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)?

Matthijs





--
Sent from: http://python.6.x6.nabble.com/PyQt-f1792048.html
_______________________________________________
PyQt mailing list    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt
Reply | Threaded
Open this post in threaded view
|

Re: sip: Accessing members results in incorrect/missing data

Phil Thompson-5
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    [hidden email]
https://www.riverbankcomputing.com/mailman/listinfo/pyqt