Skip to content

CachedThreadScheduler should wait until the previous action (if any) … #4231

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

Merged
merged 2 commits into from
Jul 23, 2016

Conversation

csabakos-zz
Copy link

…completes before releasing a worker to the pool. Fixes #4230.

…completes before releasing a worker to the pool. Fixes ReactiveX#4230.
@codecov-io
Copy link

codecov-io commented Jul 23, 2016

Current coverage is 84.14% (diff: 100%)

Merging #4231 into 1.x will decrease coverage by 0.14%

@@                1.x      #4231   diff @@
==========================================
  Files           265        265          
  Lines         17314      17316     +2   
  Methods           0          0          
  Messages          0          0          
  Branches       2627       2627          
==========================================
- Hits          14595      14571    -24   
- Misses         1875       1890    +15   
- Partials        844        855    +11   

Powered by Codecov. Last update 2284d4f...6afe4e7

pool.release(threadWorker);

// Release the worker _after_ the previous action (if any) has completed
threadWorker.schedule(new Action0() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EventLoopWorker can implement Action0 and schedule itself to release.

Copy link
Author

@csabakos-zz csabakos-zz Jul 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akarnokd What would be the advantage of doing so?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It saves an allocation; I believe io() workers are used and unsubscribed frequently enough to save on this allocation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, let me change it.

@akarnokd
Copy link
Member

👍

Given a misbehaving task, this at least prevents other work to be scheduled on the same pool.

@akarnokd akarnokd merged commit 99abbf9 into ReactiveX:1.x Jul 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants