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

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.