Skip to content

Commit 15e0aba

Browse files
aksOpsclaude
andcommitted
refactor: use try-with-resources for executor via BoundedExecutor wrapper
SonarCloud flagged ExecutorService not closed in finally clause. Can't use standard try-with-resources because ExecutorService.close() hangs up to 24 hours on stuck ANTLR threads. Fix: BoundedExecutor record wraps ExecutorService as AutoCloseable with bounded shutdown (10s graceful + 5s forced). All 3 analysis paths now use try-with-resources: try (var executor = createExecutor(parallelism)) Removes the manual shutdownExecutor() calls and finally blocks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent e6ec6c4 commit 15e0aba

1 file changed

Lines changed: 33 additions & 34 deletions

File tree

src/main/java/io/github/randomcodespace/iq/analyzer/Analyzer.java

Lines changed: 33 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -252,11 +252,7 @@ private AnalysisResult runWithCache(Path root, Integer parallelism, AnalysisCach
252252
var cacheHitsCounter = new java.util.concurrent.atomic.AtomicInteger(0);
253253

254254
final DetectorRegistry detectorRegistry = effectiveRegistry;
255-
var executorService = parallelism != null && parallelism > 0
256-
? Executors.newFixedThreadPool(parallelism, Thread.ofPlatform().daemon(true).factory())
257-
: Executors.newVirtualThreadPerTaskExecutor();
258-
try {
259-
var executor = executorService;
255+
try (var executor = createExecutor(parallelism)) {
260256
List<Future<?>> futures = new ArrayList<>(files.size());
261257
for (int i = 0; i < files.size(); i++) {
262258
final int idx = i;
@@ -309,8 +305,6 @@ private AnalysisResult runWithCache(Path root, Integer parallelism, AnalysisCach
309305
log.warn("Analysis interrupted for {}", files.get(i).path());
310306
}
311307
}
312-
} finally {
313-
shutdownExecutor(executorService);
314308
}
315309

316310
if (cache != null && cacheHitsCounter.get() > 0) {
@@ -537,10 +531,7 @@ private AnalysisResult runBatchedWithCache(Path root, Integer parallelism, int b
537531
cache.clear();
538532
}
539533

540-
var batchExecutorService = parallelism != null && parallelism > 0
541-
? Executors.newFixedThreadPool(parallelism, Thread.ofPlatform().daemon(true).factory())
542-
: Executors.newVirtualThreadPerTaskExecutor();
543-
try {
534+
try (var batchExecutor = createExecutor(parallelism)) {
544535
List<DiscoveredFile> batch = new ArrayList<>(batchSize);
545536
for (int fileIdx = 0; fileIdx < files.size(); fileIdx++) {
546537
batch.add(files.get(fileIdx));
@@ -559,7 +550,7 @@ private AnalysisResult runBatchedWithCache(Path root, Integer parallelism, int b
559550
for (int i = 0; i < batch.size(); i++) {
560551
final int idx = i;
561552
final DiscoveredFile file = batch.get(idx);
562-
futures.add(batchExecutorService.submit(() -> {
553+
futures.add(batchExecutor.submit(() -> {
563554
if (incremental) {
564555
try {
565556
Path absPath = root.resolve(file.path());
@@ -665,8 +656,6 @@ private AnalysisResult runBatchedWithCache(Path root, Integer parallelism, int b
665656
batch.clear();
666657
}
667658
}
668-
} finally {
669-
shutdownExecutor(batchExecutorService);
670659
}
671660

672661
if (cacheHits > 0) {
@@ -819,16 +808,11 @@ private AnalysisResult runSmartWithCache(Path root, Integer parallelism, int bat
819808
Map<String, Integer> edgeBreakdown = new HashMap<>();
820809
Map<String, Integer> frameworkBreakdown = new HashMap<>();
821810

822-
var executorService = parallelism != null && parallelism > 0
823-
? Executors.newFixedThreadPool(parallelism, Thread.ofPlatform().daemon(true).factory())
824-
: Executors.newVirtualThreadPerTaskExecutor();
825-
826811
// Process modules in sorted order for determinism
827812
List<String> sortedModuleKeys = new ArrayList<>(modules.keySet());
828813
sortedModuleKeys.sort(String::compareTo);
829814

830-
try {
831-
var executor = executorService;
815+
try (var executor = createExecutor(parallelism)) {
832816
List<DiscoveredFile> pendingBatch = new ArrayList<>(batchSize);
833817
int moduleIndex = 0;
834818

@@ -868,7 +852,7 @@ private AnalysisResult runSmartWithCache(Path root, Integer parallelism, int bat
868852
pendingBatch.add(file);
869853
if (pendingBatch.size() >= batchSize) {
870854
batchNumber++;
871-
var batchResult = processSmartBatch(pendingBatch, root, executor,
855+
var batchResult = processSmartBatch(pendingBatch, root, executor.delegate(),
872856
detectorRegistry, infraRegistry, incremental, cache,
873857
nodeBreakdown, edgeBreakdown, frameworkBreakdown,
874858
batchNumber, report);
@@ -884,7 +868,7 @@ private AnalysisResult runSmartWithCache(Path root, Integer parallelism, int bat
884868
// Flush remaining files
885869
if (!pendingBatch.isEmpty()) {
886870
batchNumber++;
887-
var batchResult = processSmartBatch(pendingBatch, root, executor,
871+
var batchResult = processSmartBatch(pendingBatch, root, executor.delegate(),
888872
detectorRegistry, infraRegistry, incremental, cache,
889873
nodeBreakdown, edgeBreakdown, frameworkBreakdown,
890874
batchNumber, report);
@@ -894,8 +878,6 @@ private AnalysisResult runSmartWithCache(Path root, Integer parallelism, int bat
894878
cacheHits += batchResult[3];
895879
pendingBatch.clear();
896880
}
897-
} finally {
898-
shutdownExecutor(executorService);
899881
}
900882

901883
if (filesSkipped > 0) {
@@ -1197,21 +1179,38 @@ DetectorResult analyzeFileWithRegistry(DiscoveredFile file, Path repoPath,
11971179
* minified files without .min suffix, e.g. webpack output named app.js or vendor.js)</li>
11981180
* </ol>
11991181
*/
1200-
private static void shutdownExecutor(java.util.concurrent.ExecutorService executor) {
1201-
executor.shutdown();
1202-
try {
1203-
if (!executor.awaitTermination(10, java.util.concurrent.TimeUnit.SECONDS)) {
1204-
executor.shutdownNow();
1205-
if (!executor.awaitTermination(5, java.util.concurrent.TimeUnit.SECONDS)) {
1206-
log.warn("Executor did not terminate cleanly; stuck ANTLR threads will be reclaimed at JVM exit");
1182+
/**
1183+
* Wrapper around ExecutorService that implements AutoCloseable with a bounded
1184+
* shutdown — prevents the default close() from hanging up to 24 hours on stuck
1185+
* ANTLR threads.
1186+
*/
1187+
private record BoundedExecutor(java.util.concurrent.ExecutorService delegate) implements AutoCloseable {
1188+
<T> Future<T> submit(java.util.concurrent.Callable<T> task) { return delegate.submit(task); }
1189+
1190+
@Override
1191+
public void close() {
1192+
delegate.shutdown();
1193+
try {
1194+
if (!delegate.awaitTermination(10, java.util.concurrent.TimeUnit.SECONDS)) {
1195+
delegate.shutdownNow();
1196+
if (!delegate.awaitTermination(5, java.util.concurrent.TimeUnit.SECONDS)) {
1197+
log.warn("Executor did not terminate cleanly; stuck ANTLR threads will be reclaimed at JVM exit");
1198+
}
12071199
}
1200+
} catch (InterruptedException e) {
1201+
delegate.shutdownNow();
1202+
Thread.currentThread().interrupt();
12081203
}
1209-
} catch (InterruptedException e) {
1210-
executor.shutdownNow();
1211-
Thread.currentThread().interrupt();
12121204
}
12131205
}
12141206

1207+
private BoundedExecutor createExecutor(Integer parallelism) {
1208+
var exec = parallelism != null && parallelism > 0
1209+
? Executors.newFixedThreadPool(parallelism, Thread.ofPlatform().daemon(true).factory())
1210+
: Executors.newVirtualThreadPerTaskExecutor();
1211+
return new BoundedExecutor(exec);
1212+
}
1213+
12151214
private boolean isMinified(DiscoveredFile file, String content) {
12161215
String name = file.path().getFileName().toString();
12171216
boolean nameHint = name.endsWith(".min.js") || name.endsWith(".bundle.js")

0 commit comments

Comments
 (0)