Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ObserveOn: Unsubscribe of RecursiveScheduler #3002

Closed
benjchristensen opened this issue Jun 2, 2015 · 7 comments
Closed

ObserveOn: Unsubscribe of RecursiveScheduler #3002

benjchristensen opened this issue Jun 2, 2015 · 7 comments
Labels
Milestone

Comments

@benjchristensen
Copy link
Member

Need to explore the unsubscribe of recursiveScheduler:

It seems to wrap with child.add(scheduledUnsubscribe) and then defeat that with child.add(recursiveScheduler).

@akarnokd
Copy link
Member

akarnokd commented Jun 2, 2015

Yes, looks odd. I can't really remember why the unsubscription of the worker had to be scheduled. Maybe it had something to do with pitfall no. 2: unsubscribing the downstream: something around subscribe and SafeSubscriber doing this and cutting off the last few events from the wrapped Subscriber.

@davidmoten
Copy link
Collaborator

Might be another problem here that the call to child.setProducer is made before adding all the subscriptions. I might test with #2997.

@benjchristensen
Copy link
Member Author

why the unsubscription of the worker had to be scheduled

Was it was because we could end up not emitting an onError that was scheduled but then dropped because the worker got unsubscribed?

@davidmoten
Copy link
Collaborator

I think @akarnokd is right, that SafeSubscriber reports onCompleted to its wrapped subscriber then unsubscribes which if child.add(recursiveScheduler) is present can cut short the schedule in OperatorObserveOn so that it doesn't emit its stuff.

Re onError getting dropped this is still a possiblity but still consistent with the Observable contract (?). All the scheduled polls of the queue do check for error first so that once a scheduled task is run the error can shortcut the queue. Perhaps we can do something to aid this scenario without stuffing up the onComplete case.

@davidmoten
Copy link
Collaborator

I've probably got my last comment backwards. I was thinking that SafeSubscriber was upstream of observeOn but should be downstream so wouldn't have the effect I stated.

@akarnokd
Copy link
Member

akarnokd commented Jun 3, 2015

SafeSubscriber came into play earlier when the operators used subscribe() to attach to each other instead of unsafeSubscribe().

@akarnokd
Copy link
Member

Closing via #3682.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants