-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Comments
@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. |
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 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. |
I have a |
Resolved in #2579 |
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. |
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 aFuture
. This in turn is used byScheduledAction
which is used when scheduling work via aScheduler
.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 ofcancel(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 theSubscription
itself is the cancellation token that work should be checking (most code doesn't properly useThread.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.The text was updated successfully, but these errors were encountered: