Stream memory leak

[alebianco]

Streams are not clearing some object created every time a then() is called on them, leaving 2 Promises 1 Deferred and 1 Stream in memory, even after calling detachStream or unlink.

I traced the issue to be the then(f) of the _end_promise of the Stream, which is never removed from its _update array.

When forcefully clearing the _update array those objects are garbage-collected as expected but I guess that could lead to other bugs.

This is the gist of my test https://gist.github.com/alebianco/5a4ab56a6934bf3e2ed2 I'm compiling for JS and did my profiling in Chrome.

I will try and propose a pull-request to fix this but if you have any suggestion on how this is supposed to work, it would be appreciated.

[jdonaldson]

thanks for the heads up on this.

[dionjwa]

On this topic, in general how are streams and promises cleaned up, memory wise. E.g. if you add a then(...) to a promise, and the parent promise object cluster gets disposed of, do you have to take care of unlinking your then(..) addition?

For example, with events there's a 'once' method that cleans up the listener after the event fires once. Is this necessary with promises?

[jdonaldson]

Promises maintain their connection upstream by design, since they're geared to catch multiple resolves (and throw an error in that case).

It looks like I just need to do some housecleaning for the _end array when the stream is detached.

[alebianco]

I do have a doubt about the fact that promises are upstream-linked. How do you handle throwaway promises? something like:

function connect():Promise<Bool> {
  var d = new DeferredPromise<Bool>();
  // do connection stuff and resolve
  return d;
}

// in another class
connect().then(doStuff);

the use of "doStuff" will create a linkage that will block the garbage collector to free some objects but i can't figure out a sensible way of unlinking the promise generated by then() so I've no way (that I can think of) of taking care of it ...

Am I missing something obvious?

[jdonaldson]

The promise object is permanently bound to the callback by design. Stream has a method of unbinding, and is better for situations where you might want to listen to a global emitter that will cause the async to never be gc'ed (like setTimeout).

Could you give some more examples of where you run into problems? I'm making a mostly philosophical argument here, if there's something that's causing practical problems, I'll find a way to get rid of them.

[alebianco]

I gave it a shot at fixing the leak in the Stream class. So far seems ok but my test was in no way comprehensive. Care to pitch in and review it?

[jdonaldson]

Sure, I"ll take a look