haxetink / tink_core

Core utilities
https://haxetink.github.io/tink_core
MIT License
116 stars 33 forks source link

Memory leak for Signal#select? #164

Open kevinresol opened 3 years ago

kevinresol commented 3 years ago

v2.0.2 The following code will go OOM in a few seconds.

using tink.CoreApi;

class Main {
    static final root:Signal<Int> = Signal.trigger();

    static function main() {
        new haxe.Timer(100).run = () -> {
            for (i in 0...1000000)
                root.select(v -> v == 0 ? Some(v) : None);

            trace('heapUsed: ' + Std.int(js.Node.process.memoryUsage().heapUsed / 1024 / 1024) + 'MB');
        }
    }
}

In contrast, if I do it manually with new Signal(cb -> root.handle(v -> if(v == 0) cb(v))); it seems to work fine.

I wonder if I am using it incorrectly or I wrongly expected it to be lazy/suspendable?

kevinresol commented 3 years ago

I noticed that .select is build on top of .over which links the derived signal to the root one. So technically this is not a leak but the memory is held by the root signal. I wonder if that is only useful when the root signal is more short-lived than the derived ones?

In my use case, the root signal is almost static and will stay alive throughout the app's lifespan. That seems to hold alive all the derived signals, causing the leak.

kevinresol commented 3 years ago

Should WeakRef be used for the ondispose hook so it doesn't keep the derived signal alive?