first look at PolynomB in *LinearPass

[swhite2401]

This branches fixes memory leaks in *LinearPass relating to the property K pointing to PolynomB[1].
Changes should be transparent for matlab, @lfarv could you please confirm?

[lfarv]

@swhite2401 :

1. There is one small difference, for both Matlab and python: in the original version, we look 1st at attribute "K", and if it is not available, then at "PolynomB". In the modified version, you look 1st at "PolynomB", and then at "K". In python, "K" is derived from "PolynomB", so it does not matter. In Matlab, it may make a difference if K and PolynomB are inconsistent, but this is an error, it should not happen… So no problem for me.
2. Now you avoid calling atGetOptionalDouble(ElemData,"K",0) if PolynomB is defined, but this raises a much more severe problem: if all atGetSomething functions block a reference, we have a big problem…

Apparently, indeed, PyObject_GetAttrString returns a 'new reference', so we should DECREF each attribute after use. For "scalar" values, we can do it immediately in the atGetSomething function. But for arrays where we return a pointer, we must keep the reference until atpass is called again with a "new lattice". And we don't keep the pointers to the attribute PyObjects…

We could anyway release immediately the attribute objects, and hope that no python piece of code, called by 'atpass' removes any element attribute until the tracking is finished (it would make no sense, but who knows…).

Or better we could keep in atpass a reference to the element itself, and DECREF immediately its attribute objects. Then we are protected from the case where the element is removed from the Lattice while tracking goes on, or even if the Lattice itself is removed. Remains the case where someone (python code) would remove explicitly an attribute from an element during tracking.

Summary: for me you can merge, but this only solves a very small part of the problem!

[swhite2401]

thanks @lfarv, I have no better solution for the more general problem although the situation you describe is quite unlikely (but who knows as you say...).
I can leave this branch open for discussion and potential improvements, these passmethods are not used that much anymore, I did run into this by chance so there is no rush to merge.
Maybe @willrogers would have a good proposal?

[swhite2401]

Should I merge this one too before #296 ?

[lfarv]

The memory leak problem is more generally solved in #299

[willrogers]

But presumably we want the logic here anyway. On 19 Sep 2021 14:51, Laurent Farvacque ***@***.***> wrote: The memory leak problem is more generally solved in #299<#299> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#290 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AASTW55EBCUVNVN4FP3N7B3UCXTDZANCNFSM5DYYBFJA>. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

…

-- This e-mail and any attachments may contain confidential, copyright and or privileged material, and are for the use of the intended addressee only. If you are not the intended addressee or an authorised recipient of the addressee please notify us of receipt by returning the e-mail and do not use, copy, retain, distribute or disclose the information in or attached to the e-mail. Any opinions expressed within this e-mail are those of the individual and not necessarily of Diamond Light Source Ltd. Diamond Light Source Ltd. cannot guarantee that this e-mail or any attachments are free from viruses and we cannot accept liability for any damage which you may sustain as a result of software viruses which may be transmitted in or with the message. Diamond Light Source Limited (company no. 4375679). Registered in England and Wales with its registered office at Diamond House, Harwell Science and Innovation Campus, Didcot, Oxfordshire, OX11 0DE, United Kingdom

[lfarv]

This PR is replaced by #299 and can be closed.

[swhite2401]

Very nice, however I agree with @willrogers that the logic is better this way around as it is more coherent with other passmethods. I changed the title to avoid confusion.
Unless you have objection I will merge this one today