Skip to content

Commit dd9092b

Browse files
aalhossaryaalhossary
committed
Keep the ECOD read lock balanced when loading fails
getDomainsForPdb and getAllDomains release the read lock inside their try block, so that the loader they call can take the write lock, and re-acquire it afterwards. When that loader throws, the re-acquisition never happens, and the outer finally unlocks a lock the thread no longer holds. The resulting IllegalMonitorStateException is thrown from a finally block, so it supersedes the IOException that actually caused the failure. Last night's nightly showed this: one upstream change - ECOD now answers with a 308 that HttpURLConnection will not follow - produced eight failures with three different-looking causes, only three of which named the redirect. Re-acquiring in a finally of its own keeps the lock balanced on both paths, so the original exception propagates intact.
1 parent 1fdfd2e commit dd9092b

1 file changed

Lines changed: 18 additions & 6 deletions

File tree

‎biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,15 @@ public List<EcodDomain> getDomainsForPdb(String id) throws IOException {
137137
// unlock to allow ensureDomainsFileInstalled to get the write lock
138138
logger.trace("UNLOCK readlock");
139139
domainsFileLock.readLock().unlock();
140-
indexDomains();
141-
domainsFileLock.readLock().lock();
142-
logger.trace("LOCK readlock");
140+
try {
141+
indexDomains();
142+
} finally {
143+
// re-acquire even if indexing failed, so the outer finally has a
144+
// lock to release; otherwise IllegalMonitorStateException replaces
145+
// the real cause and the failure becomes unreadable
146+
domainsFileLock.readLock().lock();
147+
logger.trace("LOCK readlock");
148+
}
143149
}
144150

145151
PdbId pdbId = null;
@@ -244,9 +250,15 @@ public List<EcodDomain> getAllDomains() throws IOException {
244250
// unlock to allow ensureDomainsFileInstalled to get the write lock
245251
logger.trace("UNLOCK readlock");
246252
domainsFileLock.readLock().unlock();
247-
ensureDomainsFileInstalled();
248-
domainsFileLock.readLock().lock();
249-
logger.trace("LOCK readlock");
253+
try {
254+
ensureDomainsFileInstalled();
255+
} finally {
256+
// re-acquire even if the download failed, so the outer finally has a
257+
// lock to release; otherwise IllegalMonitorStateException replaces
258+
// the real cause and the failure becomes unreadable
259+
domainsFileLock.readLock().lock();
260+
logger.trace("LOCK readlock");
261+
}
250262
}
251263
return allDomains;
252264
} finally {

0 commit comments

Comments
 (0)