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

Adding ScheduledAction to the official API #2579

Closed
wants to merge 5 commits into from

Conversation

akarnokd
Copy link
Member

@akarnokd akarnokd commented Feb 2, 2015

I've copied the internal ScheduledAction into the public rx.schedulers package to allow custom Scheduler implementors in other projects to take advantage of its correct unsubscription management.

I've removed the internal ScheduledAction. Since relying on internal features are discouraged anyway, use places outside RxJava need to be updated. Deprecating it doesn't work because the class is final and the internals now return the new rx.schedulers.ScheduledAction and would result in ClassCastException anyway.

I've introduced a system-wide parameter io.reactivex.scheduler.interrupt-on-unsubscribe to enable interruption globally. In addition, each ScheduledAction has its own setInterruptOnUnsubscribe which allows enabling/disabling interrupts on an individual basis (overrides the default from the system-wide parameter above for the instance).

I've also added detailed documentation to it and (while I was in the mood) to the NewThreadWorker.

There is a slight performance drawback now since setting the current thread id at the beginning of the run() method has to preserve the interruptible flag thus it has to perform a CAS instead of a 32-bit lazySet. Also note that the instance size change is unavoidable because a reference+int has the same allocation cost as a single long (there is no byte-atomics).

@akarnokd
Copy link
Member Author

akarnokd commented Feb 3, 2015

I'll post a new fresh version of this soon.

@benjchristensen
Copy link
Member

While you're at it can you rebase all the commits?

@akarnokd
Copy link
Member Author

akarnokd commented Feb 3, 2015

See rebased PR #2592.

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

Successfully merging this pull request may close these issues.

2 participants