[Improvement] Ensure JobManager.close() always shuts down background executors when jobExecutor.close() fails
#10 272 ouverte le 6 mars 2026
Métriques du dépôt
- Stars
- (3 058 stars)
- Métriques de merge PR
- (Métriques PR en attente)
Description
What would you like to be improved?
JobManager.close() calls jobExecutor.close() before shutting down statusPullExecutor and cleanUpExecutor. If jobExecutor.close() throws IOException, method execution exits early, and both schedulers are left running. This can leak background threads during shutdown, causing incomplete cleanup.
How should we improve?
Make shutdown resilient by guaranteeing executor shutdown in a finally block. Capture any IOException from jobExecutor.close(), run statusPullExecutor.shutdownNow() and cleanUpExecutor.shutdownNow() unconditionally, then rethrow the original exception . This preserves failure signaling while ensuring cleanup always runs.
Here's a test to help:
@Test
public void testCloseShouldShutdownExecutorsWhenJobExecutorCloseFails() throws IOException {
JobExecutor failingJobExecutor = Mockito.mock(JobExecutor.class);
doThrow(new IOException("close failed")).when(failingJobExecutor).close();
JobManager manager = new JobManager(config, entityStore, idGenerator, failingJobExecutor);
try {
Assertions.assertThrows(IOException.class, manager::close);
Assertions.assertTrue(manager.statusPullExecutor.isShutdown());
Assertions.assertTrue(manager.cleanUpExecutor.isShutdown());
} finally {
manager.statusPullExecutor.shutdownNow();
manager.cleanUpExecutor.shutdownNow();
}
}