Fix for #2191 - OperatorMulticast fails to unsubscribe from source

[duncani]

OperatorMulticast fails to unsubscribe from source. #2191

[benjchristensen]

The multicast operator was removed from the public API before releasing 1.0. These internal classes will eventually go away as well.

[zsxwing]

@benjchristensen OperatorMulticast is still used in replay.

[benjchristensen]

I know. But it will eventually be removed.

[duncani]

Fair enough, but in the meantime, we're still seeing thousands of Threads
being created only to be used once and never released. Could we define
"eventually"? :)

On 15 January 2015 at 16:33, Ben Christensen [email protected]
wrote:

I know. But it will eventually be removed.

—
Reply to this email directly or view it on GitHub
#2455 (comment).

Don't let your mind wander -- it's too little to be let out alone.

[benjchristensen]

Couple months :-)

Sorry, I'm not suggesting we don't fix it now. I did a poor job of setting context for this code :-)

[zsxwing]

@duncani could you update the title? You don't need to put the link in the title. Just refer to the issue number, such as Fix for #2191 - OperatorMulticast fails to unsubscribe from source.

[duncani]

Ok, given that multicast is going away, I've moved the test cases over to OperatorReplayTest (since that's what they're really testing anyway). They'll still be a valid test for however replay gets reimplemented.

[benjchristensen]

Great, that's the type of decisions I was trying to influence by mentioning that OperatorMulticast would be removed. Thank you @duncani.

[akarnokd]

I'm not sure why multicast would need to unsubscribe if the upstream completes: usually, the upstream goes away and as long as you don't retain a reference to the multicast operator itself, this shouldn't leak anything. Could you enlighten me?

[duncani]

Specifically, when employing a Scheduler, it is registered as a subscription on the Subscriber. If you don't unsubscribe, the Scheduler is never released. In the case of the current computation scheduler, nothing much is lost. However, this is fatal for the CachedThreadScheduler.

[akarnokd]

Thanks I see it now.

[akarnokd]

Let's see if this still fails.