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

Thread Interruption on Unsubscribe #1914

Closed
benjchristensen opened this issue Dec 1, 2014 · 6 comments
Closed

Thread Interruption on Unsubscribe #1914

benjchristensen opened this issue Dec 1, 2014 · 6 comments
Milestone

Comments

@benjchristensen
Copy link
Member

A few issues recently have been related to Thread interruption and brings into question why we interrupt the thread when we unsubscribe a scheduled action (via a Scheduler).

The code that causes this is from https://github.com/ReactiveX/RxJava/blob/1.x/src/main/java/rx/subscriptions/Subscriptions.java#L102 that wraps a Subscription around a Future. This in turn is used by ScheduledAction which is used when scheduling work via a Scheduler.

Issue #1898 is working to not cause an interrupt after successful termination, but I want to question why we ever interrupt as default behavior.

I don't think much thought went into choosing to call cancel(true) instead of cancel(false) and I'd like to discuss changing this to not interrupt.

The implication of interrupting are difficult to manage well and add a lot of complexity.

The only time interrupting seems wanted is if unsubscribing early to a ScheduledAction doing blocking IO. Otherwise the Subscription itself is the cancellation token that work should be checking (most code doesn't properly use Thread.isInterrupted() anyways so it rarely helps).

If an Observable is doing blocking IO it seems it should opt itself into doing Thread interruption. Perhaps we should figure out an idiomatic approach to that for opting into that behavior without requiring interruption all the time.

@daschl
Copy link
Contributor

daschl commented Dec 1, 2014

@benjchristensen makes total sense. I'm not so into the details, maybe we can only do the interruption on the IO scheduler? especially not on the computation one.

@akarnokd
Copy link
Member

akarnokd commented Dec 1, 2014

The developers of the JDK threadpool couldn't decide it either so they went for providing the option to the programmer. Either we don't do interrupts on the computation scheduler, but do it everywhere else; or we extend the API and inject an interruptible flag all around.

Two clarifications: ScheduledAction has its own Future wrapper; ScheduledAction wraps an Action0 and the ScheduledAction itself is not accessible inside this Action0 so it can't act as a cancellation token nor can be manipulated to gain access to the Future it holds to issue an cancel(true); one would need to exfiltrate the worker thread but then it becomes dangerous to call interrupt on it.

@akarnokd
Copy link
Member

akarnokd commented Feb 5, 2015

I have a ScheduledAction enhancements that will support customization of the interrupt behavior. See #2592.

@benjchristensen
Copy link
Member Author

Resolved in #2579

@benjchristensen benjchristensen added this to the 1.0.x milestone Aug 28, 2015
@vinc3m1
Copy link

vinc3m1 commented Feb 12, 2016

Following #2579 to #2592 to #2761 to #2772, it doesn't look like this is actually resolved, is it @akarnokd?

@akarnokd
Copy link
Member

None of the customizations made it and it seems Hystrix solved it internally. There were a few changes made to ScheduledAction so it doesn't call cancel(true) when cancelling from the same thread.

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

No branches or pull requests

4 participants