Michal Kordas
Michal Kordas

Reputation: 10925

Shut down two instances of ExecutorService

I need to properly shut down two instances of Executor Service in one method.

Here's my simplified code:

ExecutorService executor1 = Executors.newSingleThreadExecutor();
ScheduledExecutorService executor2 = Executors.newSingleThreadScheduledExecutor();
// logic here
executor1.shutdown();
executor2.shutdown();
try {
    if (!executor1.awaitTermination(1, TimeUnit.SECONDS)) {
        executor1.shutdownNow();
    }
} catch (InterruptedException ex) {
    throw new IllegalStateException(ex);
}
try {
    if (!executor2.awaitTermination(1, TimeUnit.SECONDS)) {
        executor2.shutdownNow();
    }
} catch (InterruptedException ex) {
    throw new IllegalStateException(ex);
}

InterruptedException is converted to IllegalStateException as I don't expect any interruptions here and this would mean my application went into illegal state.

I see one flaw in this solution - whenever first executor while shutting down throws exception, the second executor won't be properly closed. What should be correct approach here? How to safely close two instances of ExecutorService?

I'd rather like to avoid nested try-finally blocks, as I might need to add third executor service and code would become unmanageable.

Upvotes: 3

Views: 572

Answers (1)

dhke
dhke

Reputation: 15388

As for a similar situation:

Apache Commons IO has a closeQuietly() that closes streams (or rather any Closeable) while ignoring any exception during close.

public void shutdownQuietly(ExecutorService executor)
{
    try {
        if (!executor.awaitTermination(1, TimeUnit.SECONDS)) {
            executor.shutdownNow();
        }
    } catch (InterruptedException ex) {
       /* IGNORE */
    }  
}

If you need those exception, you can try some slightly more evil trickery:

class MultiExecutorShutdown
{
     private final List<InterrupedException> exceptions = new ArrayList<>();

     public void shutdown(ExecutorService service)
     {
         try {
             if (!executor.awaitTermination(1, TimeUnit.SECONDS)) {
                executor.shutdownNow();
             }
         } catch (InterruptedException ex) {
             exceptions.add(ex);
         }
     }

     public Optional<InterruptedException> getLastException()
     {
         if (exceptions.isEmpty()) {
            return Optional.empty();
         } else {
             return exceptions.get(exceptions.size() - 1);
         }
     }

     public Optional<InterruptedException> getFirstException()
     {
         if (exceptions.isEmpty()) {
            return Optional.empty();
         } else {
             return exceptions.get(0);
         }
     }
}


[...]
MultiExecutorShutdown multiShutdown = new MultiExecutorShutdown();
multiShutdown.shutdown(executor1);
multiShutdown.shutdown(executor2);
multiShutdown.shutdown(executor3);

Optional<InterruptedException> exception = multiShutdown.getLastException();
// alternative:
// Optional<InterruptedException> exception = multiShutdown.getFirstException();

if (exception.isPresent()) {
   throw new IllegalStateException(exception.get());
}

If you also need the executor which failed, you can also modify MultiExecutorShutdown to keep an (ordered) map ExecutorService -> Exception.

You can also push the throw into MultiExecutorShutdown itself, making it even more usable. And finally the whole thing can --of course-- be abstracted so that it takes a functional, calls that and records any exceptions thrown.

Upvotes: 2

Related Questions