From dd9092b5301dca5eefb9798ff9f92eebc7720644 Mon Sep 17 00:00:00 2001 From: Amr ALHOSSARY Date: Sun, 30 Aug 2026 07:41:52 -0400 Subject: [PATCH] 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. --- .../nbio/structure/ecod/EcodInstallation.java | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java index 5bd59dd14b..aeb1c50ee5 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java @@ -137,9 +137,15 @@ public List getDomainsForPdb(String id) throws IOException { // unlock to allow ensureDomainsFileInstalled to get the write lock logger.trace("UNLOCK readlock"); domainsFileLock.readLock().unlock(); - indexDomains(); - domainsFileLock.readLock().lock(); - logger.trace("LOCK readlock"); + try { + indexDomains(); + } finally { + // re-acquire even if indexing failed, so the outer finally has a + // lock to release; otherwise IllegalMonitorStateException replaces + // the real cause and the failure becomes unreadable + domainsFileLock.readLock().lock(); + logger.trace("LOCK readlock"); + } } PdbId pdbId = null; @@ -244,9 +250,15 @@ public List getAllDomains() throws IOException { // unlock to allow ensureDomainsFileInstalled to get the write lock logger.trace("UNLOCK readlock"); domainsFileLock.readLock().unlock(); - ensureDomainsFileInstalled(); - domainsFileLock.readLock().lock(); - logger.trace("LOCK readlock"); + try { + ensureDomainsFileInstalled(); + } finally { + // re-acquire even if the download failed, so the outer finally has a + // lock to release; otherwise IllegalMonitorStateException replaces + // the real cause and the failure becomes unreadable + domainsFileLock.readLock().lock(); + logger.trace("LOCK readlock"); + } } return allDomains; } finally {