Skip to content

Commit 942ca98

Browse files
committed
fix(cache): extend nested-try-finally lock release to remaining AnalysisCache methods
Follow-up to 798bccf. That commit fixed UL_UNRELEASED_LOCK_EXCEPTION_PATH in AnalysisCache.removeFile and flagged the same lock-leak pattern in two other methods that SpotBugs did not surface (likely because they differ subtly in what's inside the outer try body). Both methods — storeResults (the INSERT nodes/edges path) and the replace-with-enriched-data path — had: } finally { try { conn.setAutoCommit(true); } catch (SQLException ignored) {} rwLock.writeLock().unlock(); } If conn.setAutoCommit(true) throws anything other than SQLException (RuntimeException or Error), execution exits the finally before rwLock.writeLock().unlock() runs, leaving the write lock held and freezing all subsequent cache writers. Wrap setAutoCommit in a nested try-finally so unlock is always reached: } finally { try { try { conn.setAutoCommit(true); } catch (SQLException ignored) {} } finally { rwLock.writeLock().unlock(); } } All three lock-holding paths (removeFile, storeResults, replace-with- enriched) now share the same exception-safe pattern. mvn test -Dtest= AnalysisCacheTest passes.
1 parent 7038bd9 commit 942ca98

1 file changed

Lines changed: 18 additions & 6 deletions

File tree

src/main/java/io/github/randomcodespace/iq/cache/AnalysisCache.java

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -372,10 +372,16 @@ public void storeResults(String contentHash, String filePath, String language,
372372
log.warn("Failed to store cached results for hash {}", contentHash, e);
373373
} finally {
374374
try {
375-
conn.setAutoCommit(true);
376-
} catch (SQLException ignored) {
375+
try {
376+
conn.setAutoCommit(true);
377+
} catch (SQLException ignored) {
378+
// best-effort restore; the INSERTs have already been committed or rolled back.
379+
}
380+
} finally {
381+
// Guarantee unlock even if conn.setAutoCommit throws a non-SQLException
382+
// (RuntimeException / Error). Fixes SpotBugs UL_UNRELEASED_LOCK_EXCEPTION_PATH.
383+
rwLock.writeLock().unlock();
377384
}
378-
rwLock.writeLock().unlock();
379385
}
380386
}
381387

@@ -605,10 +611,16 @@ public void replaceAll(List<CodeNode> nodes, List<CodeEdge> edges) {
605611
log.warn("Failed to replace cache with enriched data", e);
606612
} finally {
607613
try {
608-
conn.setAutoCommit(true);
609-
} catch (SQLException ignored) {
614+
try {
615+
conn.setAutoCommit(true);
616+
} catch (SQLException ignored) {
617+
// best-effort restore; the INSERTs have already been committed or rolled back.
618+
}
619+
} finally {
620+
// Guarantee unlock even if conn.setAutoCommit throws a non-SQLException
621+
// (RuntimeException / Error). Fixes SpotBugs UL_UNRELEASED_LOCK_EXCEPTION_PATH.
622+
rwLock.writeLock().unlock();
610623
}
611-
rwLock.writeLock().unlock();
612624
}
613625
}
614626

0 commit comments

Comments
 (0)