Repository navigation
Terminate owned delaying-queue executors on shutdown - #4983
brendandburns with Copilot wants to merge 4 commits into
Conversation
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Copilot The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
415cfdf to
d78659c
Compare
|
|
||
| class DefaultDelayingQueueTest { | ||
|
|
||
| private final DefaultDelayingQueue<String> queue = new DefaultDelayingQueue<>(); |
There was a problem hiding this comment.
I'm worried that this will break if the tests run in parallel. Instead of making it a member variable in the Test class, enqueue each one in a list, and tear down all of them within the teardown method.
|
@copilot please address comment |
Co-authored-by: brendandburns <5751682+brendandburns@users.noreply.github.com>
| class DefaultRateLimitQueueTest { | ||
|
|
||
| private final ExecutorService executor = Executors.newSingleThreadExecutor(); | ||
| private final DefaultRateLimitingQueue<String> rlq = |
There was a problem hiding this comment.
Same comment as above, this makes these tests not work when run in parallel. I realize that for now there is only one test, but this will be a gotcha whenever someone adds an additional test. Please move this back into the test method and add it to a queue for teardown at the end of the tests.
|
@copilot please address comment and make sure CI/CD is passing |
Co-authored-by: brendandburns <5751682+brendandburns@users.noreply.github.com>
Addressed in 9037d0a: the rate-limiting queue and executor are method-local and registered in thread-safe teardown lists. All 18 targeted tests pass with parallel execution enabled. CI for this commit is blocked: Bazel, Spotless, and CodeQL report |
Default delaying and rate-limiting queues leave executor threads alive after shutdown, potentially preventing JVM termination. Waiting tasks can also remain blocked until a distant delayed-item deadline.