further questions about improving memory leaks on `PV.disconnect()` about pyepics HOT 4 OPEN

@juanfem

I may have missed something, but it seems to me from the code that epics.PV.get_pv() does not add callbacks into epics.ca._cache

Ah, yes, that is correct. And if one does

 import epics
 p1 = epics.PV(pvname_1)
 p2 = epics.PV(pvname_1)

The ca cache for pvname_1 will have two connection callbacks and two access-rights callbacks, but the count for each PV's __on_connect and __on_access_rights_event will still be 1. So, yeah, it does seem like one would have to work really hard (or mix the PV level with lower-level ca calls) to retain references to callbacks.

And, I agree with all the rest of what you say. I was just trying to be cautious about using 'List.count()' and 'List.remove()' together -- if the count is not 1, then 'List.remove()' only removes 1. Even though it is hard to see how count could be greater than 1, I think we do mean "remove all", not "remove the first one".

from pyepics.

@newville , thanks for accepting my PR.

The reason why I assumed that the calls to count always return 1 is that the references to those callbacks are added by the __init__ method:

I didn't find any other part of the code that stores more references to the __on_connect or __on_access_rights_event methods in the cache, and given that they are private methods a user should not do it either.

But in any case, your suggestion should work and it might be a safer approach.

from pyepics.

@juanfem In principle, one can pass in a connection callback or access_rights callback with epics.PV.get_pv() which first looks for an existing PV rather than always creating a new connection. In that case, there could be more than one connection or access_rights callback.
As an example, one could be writing GUI code that has multiple places that reference a single PV. In that case, someone might want multiple connection callbacks (so the widget can enunciate a disconnected/reconnected PV, for example).

And this is what I always struggle with for PV.disconnect(): In this GUI case, if the screen gets closed, should all the PVs for that screen disconnect? To be clear, removing the first connection callback might remove the wrong one and it will drop the actual channel., so either remove None of them (and suffer the complaints about "memory leaks") or remove all of them and lose some connection callbacks.

Again, I honestly do not know what the right behaviour is.

from pyepics.

I may have missed something, but it seems to me from the code that epics.PV.get_pv() does not add callbacks into epics.ca._cache. Instead, callbacks are stored in the lists epics.PV.callbacks, epics.PV.connection_callbacks, and epics.PV.access_callbacks.
When an event occurs, epics.ca looks in its cache and calls the methods epics.PV.__on_changes(), epics.PV.__on_connect(), or epics.PV.__on_access_rights_event(), which in turn call all registered callbacks.

Those three lists that contains the callbacks are cleared by disconnect(), which calls the method epics.PV.clear_callbacks().
This is the original behavior and it remains unchanged.

The only thing I changed is that epics.PV.disconnect() now removes the references to epics.PV.__on_connect and epics.PV.__on_access_rights_event from epics.ca._cache. Without that change, the PV objects cannot be collected by the garbage collector. Moreover, for every connection or access right event, epics.ca will call the methods epics.PV.__on_connect() or epics.PV.__on_access_rights_event() for "disconnected" PV objects, which won't trigger any event because epics.PV.clear_callbacks() cleared the lists.
Regarding the callback epics.PV.__on_changes(), it was already cleared by epics.PV._clear_auto_monitor_subscription().

Coming back to the original question here, I only see two options for the if count in

from pyepics.