Possible false positive in java:S2095

Rules and Languages Report False-positive / False-negative...
, ,
1

Environment

  • SonarQube Server: 26.4.0.121862
  • SonarJava: 8.27.0.43088
  • Java: 21.0.8
  • Build: Maven
  • Rule: java:S2095

Minimal reproducer

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

public class S2095ExecutorShutdownNow {

    static String executeWithTimeout() {
        ExecutorService executor =
                Executors.newSingleThreadExecutor(
                        Thread.ofVirtual().factory()
                );

        Future<String> future =
                executor.submit(S2095ExecutorShutdownNow::executeOperation);

        try {
            return future.get(5, TimeUnit.SECONDS);
        } catch (TimeoutException exception) {
            future.cancel(true);
            return "timeout";
        } catch (InterruptedException exception) {
            Thread.currentThread().interrupt();
            return "interrupted";
        } catch (Exception exception) {
            return "failure";
        } finally {
            executor.shutdownNow();
        }
    }

    static String executeOperation() {
        return "success";
    }
}

Actual behavior

java:S2095 reports the issue on the ExecutorService declaration:

Use try-with-resources or close this ExecutorService in a finally clause.

However, the executor is explicitly shut down in a finally block:

finally {
    executor.shutdownNow();
}

The finally block is executed on every exit path, including successful completion, timeout, interruption, and task failure.

shutdownNow() stops the executor from accepting new tasks, prevents queued tasks from starting, and attempts to interrupt actively executing tasks.

I understand that it does not wait for active tasks to terminate and that interruption is only a best-effort cancellation mechanism. An interrupt-insensitive task may continue running temporarily.

However, replacing this code with try-with-resources or ExecutorService.close() is not behaviorally equivalent. In Java 21, close() performs an orderly shutdown and waits until the executor has completely terminated. For a timeout wrapper, this could cause the method to continue waiting after the timeout enforced by Future.get(...).

In the production code, the executor is created once per invocation and is not retained for reuse. The submitted task performs an HTTP operation with explicit connection and read timeouts. When Future.get(...) times out, the future is cancelled, and shutdownNow() is then called from finally.

The use of shutdownNow() is therefore intentional: cancellation and executor shutdown are requested without extending the caller-visible timeout by waiting for complete executor termination.

Expected behavior

The analyzer should recognize shutdownNow() as an explicit executor lifecycle cleanup operation, or clarify that S2095 intentionally requires the executor to terminate completely.

The current code does not abandon the ExecutorService: shutdownNow() is called from a finally block on every control-flow path.

Could you confirm whether S2095 is expected to recognize shutdownNow() in a finally block as explicit cleanup, or whether the rule intentionally requires complete executor termination?

References

3

Hi @vxtls,

Thank you for taking the time to report this issue.

I checked the rule intent and the Java 21 ExecutorService behavior. shutdownNow() is an intentional shutdown step, but it is not equivalent to close(). In Java 21, close() waits for the executor to terminate, while shutdownNow() is best-effort and can return while tasks are still running.

Because of that, I do not see this as a confirmed false positive for java:S2095. Your point that replacing this code with close() would change the timeout behavior is valid, but that also means the executor may still be alive after the method returns, which is what the rule is conservative about.

If this non-blocking shutdown is intentional in your code, the practical option today is to suppress the issue at that location. If you can share a case where the executor is guaranteed terminated without close() or waiting for termination, we can take another look.

Thanks again for your report.
Erwan