globaltcad / sprouts

A property MVVM API for the Swing-Tree library
MIT License
1 stars 0 forks source link

Make Event Registration API Less Susceptible to Memory Leaks #141

Open Gleethos opened 4 days ago

Gleethos commented 4 days ago

A continuation of the discussion at: https://github.com/globaltcad/swing-tree/issues/212

Gleethos commented 2 days ago

After some short discussions and a bit of contemplating there might be a simpler solution for this problem which is a bit of a compromise between the radical suggestion of hiding all change listener registration sites or making nothing weakly referenced and embracing manual memory management by the user.

The way property views behave is that they are weakly referenced by the property they are viewing. And so they get collected alongside their change listeners whenever the user no longer holds a reference to it. I think this is actually already a really nice compromise between user expectation and memory safety, because the behavior is much more intuitive.

So this also means that the following listener will only work temporarily:

myModel.myProperty().view( ... ).onChange( ...capturing lots of references... );

If you want to keep the updates from the registered listener, then you have to hold onto the view! This API prevents memory leaks and it is also not terrible to work with. What is still problematic then is the fact that you can still register change listeners directly on properties:

myModel.myProperty().onChange( ...capturing lots of references... );

This will stick around forever, if not removed manually at some later point in time.

This is obviously problematic, because a syntactically preferable lambda expression will always leak memory! And the alternative is to design the usage site to have a life cycle, which is a really hard thing to since in Java the lifecycle of an object is managed by the GC. A user often does not know the lifecycle of their objects...


So a good solution to this whole situation might be to just make property view the only place where to register listeners! So we change the design of the API so that regular properties (Val and Var) don't have the methods, but the views expose a special interface which has these methods.

So you would then only be able to register a listener like this:

myModel.myProperty().view(it->it).onChange( ...capturing lots of references... );

(Here we create a no-op view, we might also want to have a view() method for that)

And just like above, we have the simple rule: You want to receive change event callbacks -> then keep a reference to the property view!


Now here how this design looks like in principle:

flowchart TD
    B(Val) --> V
    B --> C(Var)

    O(Observable) --> I(Viewables)
    O --> V(Viewable)

    L(Vals) --> M(Vars)
    L --> I

Here every view(..) method, returns a Viewable on a property (Val/Var). And in case of a property list, the view methods always return a Viewables. These Viewables are always weakly referenced, and it is where a user can then register change listeners onto.