Correct Handling of `InterruptedExceptions` in Java

Question

So, I am having an issue where Sonar Qube is flagging my code due to improper handling of InterruptedExceptions. The exact Sonar Qube error is java:S2142, and can be found here: https://github.com/joansmith/sonar-java/blob/master/java-checks/src/main/resources/org/sonar/l10n/java/rules/squid/S2142.html

My code is below - Sonar Qube is flagging the handling of possible InterruptedExceptions in extractFutureIntoList() as the source of the problem:

@Service
@Log4j2
public class AsynchronousDataGrabber {

  ExecutorService executorService = Executors.newFixedThreadPool(10);

  public List<MyDataObject> getDataAsynchronously() {
    Optional<Future<MyDataObject>> future01 = getDataFuture("01");
    Optional<Future<MyDataObject>> future02 = getDataFuture("02");
    Optional<Future<MyDataObject>> future03 = getDataFuture("03");

    List<MyDataObject> list = new ArrayList();
    
    extractFutureIntoList(future01, list);
    extractFutureIntoList(future02, list);
    extractFutureIntoList(future03, list);
    
    return list;
  }


  private Optional<Future<MyDataObject>> getDataFuture(String key) {
    try {
      return Optional.of(executorService.submit(() -> getDataFromRemoteApi(key)));
    } catch(Exception e) {
      log.error("Exception", e);
      return Optional.empty();
    }
  }
  
  private void extractFutureIntoList(Optional<Future<MyDataObject>> future, List<MyDataObject> list) {
    try {
      if (future.isPresent()) {  
        // This is apparently where the `InterruptedException` can occur - Future.get()
    list.add(future.get().get());  
      }
    } catch (Exception e) {
      // SonarQube is telling me that merely logging an `InterupptedException` is not enough
      // Apparently, I must either rethrow the `InterruptedException`,
      //or call `Thread.currentThread().interrupt()
      log.error("Exception", e);
      return;   
    }
  }
}

Sonar Qube proposes that I solve this problem either by rethrowing InterruptedException, or by calling Thread.currentThread().interrupt() -- whenever Future.get() is interrupted.

My problem with this is that getDataAsynchronously() is being called by my application's main thread. If the only acceptable response to an InterruptedException is to rethrow it or interrupt the current thread, then a single interrupt in one of my executorService's threads will bring down my entire application. This seems to be an excessive response, especially given that the task being run by the threads - getDataFromRemoteApi() - may not always be successful anyway.

Is there a better way to handle InterruptedException - ideally one that is acceptable to Sonar Qube, but doesn't involve killing the thread that is trying to call Future.get()?

I have tried logging the InterruptedException (log.error("Exception", e);), and I have tried to catch and rethrow the InterruptedException (throw new RuntimeException(e);) - neither appeased Sonar Qube.

Answer 1

The catching of an InterruptedException also clears the interrupted status of the thread. If you decide to eat the InterruptedException, then in all cases I can think of you should call Thread.currentThread().interrupt() to re-mark the thread as interrupted, so that any other call on that thread to Thread.isInterrupted() will report that an interrupt occurred. The other viable alternative (which SonarQube suggests) is to change various method signatures to declare that InterruptedException can be thrown, and then don't catch the exception. Both of these approaches ensure that the fact that the thread was interrupted at some point is available to interested parties.

In almost all cases I can think of where you can't adjust method signatures, the first option is exactly what you want to do.

It sounds like for whatever (perhaps valid) reason maybe you don't want to do this, so assuming that your case is valid, I suppose you could just (as SonarQube suggests) simply log the exception without resetting the original interrupted status of the thread, but just be aware that you're losing information in this case (everything will look to other parties as if an interrupt never occurred).

Answer 2

You should never ignore InterruptedException indeed.

Risen InterruptedException means running thread was marked as interrupted, that signal (could be checked with Thread.currentThread.isInterrupted()) was received and removed. So catching it without throwing up/calling Thread.currentThread().interrupt() leads to forgetting the fact of interruption forever.

If you have caught an InterruptedException do graceful return from the method and throw exception up or recover the flag and return.

In your particular case getting an InterruptedException in the specified line means your main thread has been interrupted (not an executor's). It means somebody stopped the program, thus is not interested in method return value anymore. I suggest you either:

1. Call Thread.currentThread().interrupt() and then throw new RuntimeException(e). That's okey - you both keep the flag of interruption and indicate that your getDataAsynchronously() hasn't been executed correctly and there is no answer to return.
2. Do "best effort" - call Thread.currentThread().interrupt() and just return from extractFutureIntoList. Your program would continue executing extractFutureIntoList calls left. If any of other futures would be already complete, thus their result is available immediately, future.get() would return calculation result without throwing InterruptedException. So you would collect data from all futures done at the moment of thread interruption. It would be a kind of graceful shutdown.
3. If possible - mark getDataAsynchronously as throws InterruptedException and re-throw exception. It's a best practice - mark any blocking method as throws InterruptedException. Your method getDataAsynchronously is blocking and actually implies to throw it.