Fix issue with listeners not being cleaned up in alt'

[solatis]

The current implementation of alt' has a bug where it doesn't clean up previous listeners once one of the deferreds have resolved.

This problem can be shown using the following code:

(def control-chan (md/deferred))
(def x (md/deferred))

(def y (alt control-chan x))
(md/success! x {:foo :bar})

(println "control chan listeners: " (pr-str (.-listeners control-chan))))

You will see that, even though both x and y have realized, the listener on control-chan is still there. If you repeat this block of code many times, many listeners will remain active (we kept running into tens of thousands of runaway listeners).

This PR addresses this problem, by using listeners instead. Upon realization of the deferred returned by alt', it cancels all outstanding listeners.

[KingMob]

@solatis I would avoid doing [&args] here, since that's actually a valid symbol name. This will break if more than one arg is passed. Try something like [& _] instead.

[KingMob]

Also, if I'm not mistaken, a listener can be shared, so you can move its creation outside the loop and reuse it for each alternative.

[KingMob]

@solatis I've been diving into the code to try and understand your suggested PR.

I think there's a bug: the original code used chain', which internally calls unwrap'. This will successively unwrap nested deferreds, but your code won't. Your listener fns will be called on the wrong value if a deferred is delivered to x. It'll be passed the deferred, instead of continuing to unwrap.

E.g.:

(let [x (deferred)
      l (listener #(println "listener success" %)
                  #(println "listener error" %))]
    (on-realized (chain' x)
                 #(println "orig success" %)
                 #(println "orig error" %))
    (add-listener! x l)

    (success! x (success-deferred :foo)))

orig success :foo
listener success << :foo >>

I don't know if chain' is needed here, but I definitely think `unwrap`` is.

[KingMob]

@ztellman Does this analysis make sense to you?

[solatis]

Hi @KingMob I apologize I've been on vacation and busy with work and didn't notice your comments. Great to see things are moving forward, thanks a lot for that.

Regarding unwrap -- you're probably right. It doesn't hurt to add this anyway.

[KingMob]

Not a problem. I'm still here.

One correction to my comment above about chain': I think you definitely to need to keep the (chain' x) that's present in the original code. All we know about x when your code runs is it's an IDeferred, but add-listener can only be called on an IMutableDeferred. Wrapping it with (chain' x) ensures that the listeners would have an IMutableDeferred to attach to.

I'm not seeing any inherent reason listeners couldn't be extended to promises/futures/delays and other non-IMutableDeferreds, but that's a bigger task.

[KingMob]

@solatis Any time to work on this?