From cbc8b484a1d41050d1497f6327fa95e06c52138c Mon Sep 17 00:00:00 2001 From: Joe Wicentowski Date: Mon, 23 Mar 2026 11:46:16 -0400 Subject: [PATCH] [bugfix] Fix BrokerPool shutdown hang causing CI timeouts Three targeted fixes prevent the forked JVM from hanging after BrokerPool.shutdown() completes: 1. StatusReporter threads are now daemon threads. The startup and shutdown status reporter threads are monitoring-only and must not prevent JVM exit. Added newInstanceDaemonThread() to ThreadUtils. 2. Four wait loops in BrokerPool that swallowed InterruptedException and used unbounded wait() now have 1-second poll timeouts, isShuttingDown() checks, and proper interrupt handling: - get() service mode wait: breaks on shutdown or interrupt - get() broker availability wait: throws EXistException on shutdown - enterServiceMode() wait: breaks on shutdown or interrupt - shutdown() active brokers wait: re-sets interrupt flag and breaks 3. At end of shutdown, instanceThreadGroup.interrupt() wakes any lingering threads in the instance's thread group. Previously, 4 test classes required exclusion or timeout workarounds (DeadlockIT, RemoveCollectionIT, CollectionLocksTest, MoveResourceTest). Now all complete cleanly: 6533 unit tests + 9 integration tests, 0 failures, clean JVM exit. Affects PRs with CI timeout workarounds: #6112, #6139, #6138 Related: #3685 (FragmentsTest deadlock) Co-Authored-By: Claude Opus 4.6 (1M context) --- .../java/org/exist/storage/BrokerPool.java | 41 ++++++++++++++----- .../main/java/org/exist/util/ThreadUtils.java | 6 +++ 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/exist-core/src/main/java/org/exist/storage/BrokerPool.java b/exist-core/src/main/java/org/exist/storage/BrokerPool.java index f6f69b9554a..ac442080514 100644 --- a/exist-core/src/main/java/org/exist/storage/BrokerPool.java +++ b/exist-core/src/main/java/org/exist/storage/BrokerPool.java @@ -92,6 +92,7 @@ import static com.evolvedbinary.j8fu.fsm.TransitionTable.transitionTable; import static org.exist.util.ThreadUtils.nameInstanceThreadGroup; +import static org.exist.util.ThreadUtils.newInstanceDaemonThread; import static org.exist.util.ThreadUtils.newInstanceThread; /** @@ -543,7 +544,7 @@ private void _initialize() throws EXistException, DatabaseConfigurationException statusReporter = new StatusReporter(SIGNAL_STARTUP); statusObservers.forEach(statusReporter::addObserver); - final Thread statusThread = newInstanceThread(this, "startup-status-reporter", statusReporter); + final Thread statusThread = newInstanceDaemonThread(this, "startup-status-reporter", statusReporter); statusThread.start(); // statusReporter may have to be terminated or the thread can/will hang. @@ -1225,12 +1226,16 @@ public DBBroker get(final Optional subject) throws EXistException { //No active broker : get one ASAP while(serviceModeUser != null && subject.isPresent() && !subject.equals(Optional.ofNullable(serviceModeUser))) { + if(isShuttingDown()) { + break; + } try { LOG.debug("Db instance is in service mode. Waiting for db to become available again ..."); - wait(); + wait(1000); } catch(final InterruptedException e) { Thread.currentThread().interrupt(); - LOG.error("Interrupt detected"); + LOG.warn("Interrupted while waiting for service mode to end"); + break; } } @@ -1245,11 +1250,15 @@ public DBBroker get(final Optional subject) throws EXistException { } else //... or wait until there is one available while(inactiveBrokers.isEmpty()) { + if(isShuttingDown()) { + throw new EXistException("BrokerPool is shutting down"); + } LOG.debug("waiting for a broker to become available"); try { - this.wait(); + this.wait(1000); } catch(final InterruptedException e) { - //nothing to be done! + Thread.currentThread().interrupt(); + throw new EXistException("Interrupted while waiting for a broker", e); } } } @@ -1400,10 +1409,14 @@ public DBBroker enterServiceMode(final Subject user) throws PermissionDeniedExce synchronized(this) { if(!activeBrokers.isEmpty()) { while(!inServiceMode) { + if(isShuttingDown()) { + break; + } try { - wait(); + wait(1000); } catch(final InterruptedException e) { - //nothing to be done + Thread.currentThread().interrupt(); + break; } } } @@ -1610,7 +1623,7 @@ void shutdown(final boolean killed, final Consumer shutdownInstanceConsu statusObservers.forEach(statusReporter::addObserver); synchronized (this) { - final Thread statusThread = newInstanceThread(this, "shutdown-status-reporter", statusReporter); + final Thread statusThread = newInstanceDaemonThread(this, "shutdown-status-reporter", statusReporter); statusThread.start(); // DW: only in debug mode @@ -1637,7 +1650,9 @@ void shutdown(final boolean killed, final Consumer shutdownInstanceConsu //Wait until they become inactive... this.wait(1000); } catch (final InterruptedException e) { - //nothing to be done + Thread.currentThread().interrupt(); + LOG.warn("Interrupted while waiting for active brokers to return during shutdown"); + break; } //...or force the shutdown @@ -1745,7 +1760,13 @@ void shutdown(final boolean killed, final Consumer shutdownInstanceConsu statusReporter.terminate(); statusReporter = null; -// instanceThreadGroup.destroy(); + // interrupt any remaining threads in the instance thread group + // to prevent the JVM from hanging on non-daemon threads + try { + instanceThreadGroup.interrupt(); + } catch (final SecurityException e) { + LOG.warn("Could not interrupt instance thread group: {}", e.getMessage()); + } } } finally { status.process(Event.FINISHED_SHUTDOWN); diff --git a/exist-core/src/main/java/org/exist/util/ThreadUtils.java b/exist-core/src/main/java/org/exist/util/ThreadUtils.java index 4f9e34d15fb..525c53168e6 100644 --- a/exist-core/src/main/java/org/exist/util/ThreadUtils.java +++ b/exist-core/src/main/java/org/exist/util/ThreadUtils.java @@ -58,6 +58,12 @@ public static Thread newInstanceThread(final ThreadGroup threadGroup, final Stri return new Thread(threadGroup, runnable, nameInstanceThread(instanceId, threadName)); } + public static Thread newInstanceDaemonThread(final Database database, final String threadName, final Runnable runnable) { + final Thread thread = new Thread(database.getThreadGroup(), runnable, nameInstanceThread(database, threadName)); + thread.setDaemon(true); + return thread; + } + public static String nameGlobalThread(final String threadName) { return "global." + threadName; }