This repository has been archived by the owner on Sep 26, 2023. It is now read-only.
fix: [Approach 1]Watchdog does not shut down executor on client closing if the executor is provided by ExecutorProvider. #1875
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is an attempt to fix #1858.
Note that the issue above is just the behavior of multiple convoluted issues. The background is as below:
shutdown
fix: do not shut down executor when WatchDog is shut down #871shutdownNow
fix: Watchdog.shutdownNow() does not shutdown executor #1158In short, the root cause is that
Watchdog.shutdown
andWatchdog.isShutdown
should have consistent behavior, but we also need to decide if we want Watchdog to manage the Executor or not.There are three possible approaches:
shouldAutoClose
is the source of truth to decide if we want to shutdown executor or not, have to expose a new interface(not breaking since we can add it as a default) inWatchdogProvider
to setshouldAutoClose
. This is my preferred approach since I think it's the cleanest one and still provides the ability to auto close the executors provided to Watchdog.shouldAutoClose
andneedExecutor
to decide if we want to shutdown executor or not.