prevent threads leaking #1325
prevent threads leaking #1325
Conversation
Qodana Community for JVMIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/[email protected]
with:
upload-result: trueContact Qodana teamContact us at [email protected]
|
sergiyvamz
force-pushed
the
fix-issue-1321-thread-leaks
branch
2 times, most recently
from
0efa17b to
89f7453
Compare
| this.nodeExecutorService.shutdownNow(); | ||
| } | ||
| } catch (InterruptedException e) { | ||
| // do nothing |
There was a problem hiding this comment.
Would it make sense to call this.nodeExecutorService.shutdownNow(); here if the awaitTemrmination request is interrupted
There was a problem hiding this comment.
There's another final block in run() method that can catch InterruptedException and handle it.
sergiyvamz
force-pushed
the
fix-issue-1321-thread-leaks
branch
from
89f7453 to
88f3120
Compare
sergiyvamz
force-pushed
the
fix-issue-1321-thread-leaks
branch
from
88f3120 to
7df7774
Compare
sergiyvamz
force-pushed
the
fix-issue-1321-thread-leaks
branch
from
7df7774 to
9533dae
Compare
| this.stop.set(true); | ||
| this.shutdownNodeExecutorService(); | ||
|
|
||
| final Connection conn = this.monitoringConnection.get(); |
There was a problem hiding this comment.
@sergiyvamz would it be better to use getAndSet ? in a few places you are splitting get and set on atomic in this class...
3 participants
Summary
Use ExecutorService to manage node monitoring threads
#1321
Additional Reviewers
@karenc-bq
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.