Skip to content

Use ExecutorService instead of ScheduledExecutorService where necessary#9325

Merged
jihoonson merged 2 commits intoapache:masterfrom
mgill25:executor-service-fix
Feb 12, 2020
Merged

Use ExecutorService instead of ScheduledExecutorService where necessary#9325
jihoonson merged 2 commits intoapache:masterfrom
mgill25:executor-service-fix

Conversation

@mgill25
Copy link
Contributor

@mgill25 mgill25 commented Feb 6, 2020

Fixes #9286

Description

Changed the two instances mentioned in the PR to use Execs.singleThreaded, which returns an ExecutorService, instead of a ScheduledExecutorService.

I looked at the concurrency checklist (Threads and Executors section), but don't see anything immediate that I also need to do. Please correct me if I missed something.


This PR has:

  • been self-reviewed.

@mgill25 mgill25 closed this Feb 6, 2020
@mgill25 mgill25 reopened this Feb 6, 2020
@jihoonson
Copy link
Contributor

Hi @mgill25, thank you for your contribution. The change looks good to me, but technically this PR doesn’t fix the issue #9286 because it is about prohibiting this pattern by adding a new inspection rule. Are you interested in adding such a rule? If so please include the new rule in the PR. If not, please update the PR description to not include “Fixes #9286”.

@mgill25
Copy link
Contributor Author

mgill25 commented Feb 11, 2020

@jihoonson My bad, I'm not too familiar with IntelliJ inspection, so I guess I missed it. :)

Copy link
Contributor

@jihoonson jihoonson left a comment

Choose a reason for hiding this comment

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

@mgill25 thank you for adding the inspection rule!

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.

Prohibit assigning ScheduledExecutorService into ExecutorService

2 participants