From 5bc5d371f88c83bd2adaeac8816a10fe790525bb Mon Sep 17 00:00:00 2001 From: Shivan Taher Date: Tue, 11 Aug 2026 10:20:30 +0200 Subject: [PATCH 1/3] ARTEMIS-6179 Add removeMessage deadlock repro test Adds testRemoveMessageWhilstPagingAndConsuming, mirroring the existing testMoveMessageWhilstPagingAndConsuming/ManagementCopyThread pattern that caught the equivalent copyReference() deadlock (ARTEMIS-5376). QueueImpl#deleteReference() is still synchronized and calls iterQueue(), which locks depageLock, while QueueImpl#depage() locks depageLock first and then enters a synchronized(this) block. Racing QueueControl#removeMessage() against depaging while consuming can deadlock the two threads against each other. Detection uses the JVM's own ThreadMXBean deadlock detector instead of a fixed timeout, since a hang here is the failure itself. --- .../ManagementWithPagingServerTest.java | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ManagementWithPagingServerTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ManagementWithPagingServerTest.java index 09ddd9006bcb..875f754bddcb 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ManagementWithPagingServerTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ManagementWithPagingServerTest.java @@ -20,12 +20,16 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import org.apache.activemq.artemis.api.core.RoutingType; import org.apache.activemq.artemis.json.JsonArray; import org.apache.activemq.artemis.json.JsonNumber; import org.apache.activemq.artemis.json.JsonObject; import org.apache.activemq.artemis.json.JsonValue; +import java.lang.management.ManagementFactory; +import java.lang.management.ThreadInfo; +import java.lang.management.ThreadMXBean; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.List; @@ -48,6 +52,7 @@ import org.apache.activemq.artemis.core.server.ActiveMQServers; import org.apache.activemq.artemis.core.settings.impl.AddressFullMessagePolicy; import org.apache.activemq.artemis.core.settings.impl.AddressSettings; +import org.apache.activemq.artemis.tests.util.Wait; import org.apache.activemq.artemis.utils.RandomUtil; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -322,6 +327,72 @@ public void testMoveMessageWhilstPagingAndConsuming() throws Exception { assertEquals(messages.length, console.copiedMessages); } + /** + * Reproduction for ARTEMIS-6179: QueueImpl#deleteReference() is still {@code synchronized} and calls + * iterQueue(), which locks depageLock -- while QueueImpl#depage() locks depageLock first and then enters a + * synchronized(this) block. Racing QueueControl#removeMessage() (-> deleteReference()) against depaging + * (triggered here by the ReceiverThread acking messages) can deadlock the two threads against each other. + * This mirrors testMoveMessageWhilstPagingAndConsuming(), which caught the equivalent bug for copyReference() + * (ARTEMIS-5376), replacing copyMessage with removeMessage. Detection uses the JVM's own deadlock detector + * (ThreadMXBean) instead of a fixed timeout, since a hang here is the failure itself. + */ + @Test + public void testRemoveMessageWhilstPagingAndConsuming() throws Exception { + final int messagesPerIteration = 2000; + final int maxIterations = 20; + final long iterationTimeoutMillis = 5000; + final long pollIntervalMillis = 20; + + SimpleString address = RandomUtil.randomUUIDSimpleString(); + SimpleString queue = RandomUtil.randomUUIDSimpleString(); + + session1.createQueue(QueueConfiguration.of(queue).setAddress(address)); + + QueueControl queueControl = createManagementControl(address, queue); + + ThreadMXBean threadMXBean = ManagementFactory.getThreadMXBean(); + + // A single remover is sufficient: QueueControlImpl#removeMessage() is serialized broker-wide through + // ActiveMQServerImpl's managementLock, so extra concurrent removers would just contend on that lock + // without increasing the odds of racing depage(). + ManagementRemoveThread remover = new ManagementRemoveThread(queueControl); + remover.setDaemon(true); + remover.start(); + + long[] deadlockedIds = null; + + for (int iteration = 0; iteration < maxIterations; iteration++) { + SenderThread sender = new SenderThread(address, messagesPerIteration, 0); + ReceiverThread receiver = new ReceiverThread(queue, messagesPerIteration, 0); + sender.setDaemon(true); + receiver.setDaemon(true); + + sender.start(); + receiver.start(); + + Wait.waitFor(() -> !receiver.isAlive() || threadMXBean.findDeadlockedThreads() != null, + iterationTimeoutMillis, pollIntervalMillis); + + deadlockedIds = threadMXBean.findDeadlockedThreads(); + if (deadlockedIds != null) { + break; + } + } + + remover.exit(); + remover.join(iterationTimeoutMillis); + assertNull(remover.getError()); + + if (deadlockedIds != null) { + StringBuilder sb = new StringBuilder("Deadlock detected between removeMessage() and depage():\n"); + for (long id : deadlockedIds) { + ThreadInfo info = threadMXBean.getThreadInfo(id, Integer.MAX_VALUE); + sb.append(info).append('\n'); + } + fail(sb.toString()); + } + } + @Override @BeforeEach public void setUp() throws Exception { @@ -507,4 +578,36 @@ public void exit() { stop = true; } } + + private class ManagementRemoveThread extends Thread { + + private QueueControl queueControl; + private volatile boolean stop = false; + private Exception error = null; + + private ManagementRemoveThread(QueueControl queueControl) { + this.queueControl = queueControl; + } + + @Override + public void run() { + try { + Random random = new Random(System.currentTimeMillis()); + while (!stop) { + long messageID = random.nextInt(5000); + queueControl.removeMessage(messageID); + } + } catch (Exception e) { + error = e; + } + } + + public Exception getError() { + return error; + } + + public void exit() { + stop = true; + } + } } From fda107b4050015ac864f649f4efe1ebdd2889b4b Mon Sep 17 00:00:00 2001 From: Shivan Taher Date: Tue, 11 Aug 2026 10:20:43 +0200 Subject: [PATCH 2/3] ARTEMIS-6179 Fix deleteReference/depage deadlock Removes synchronized from QueueImpl#deleteReference(). It calls iterQueue(), which acquires depageLock internally, while depage() acquires depageLock first and then enters a synchronized(this) block. The reversed lock order deadlocks QueueControl#removeMessage() against the paging executor whenever they race on an actively paging queue. This mirrors the fix already applied to copyReference() in ARTEMIS-5376: iterQueue() already provides its own synchronization via depageLock, so the outer synchronized on deleteReference() is redundant and unsafe. Verified with testRemoveMessageWhilstPagingAndConsuming, which reliably deadlocks without this change and passes cleanly with it. --- .../org/apache/activemq/artemis/core/server/impl/QueueImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java index 04a47ad3aa4e..a96e15eac9b0 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java @@ -2223,7 +2223,7 @@ public void destroyPaging() throws Exception { } @Override - public synchronized boolean deleteReference(final long messageID) throws Exception { + public boolean deleteReference(final long messageID) throws Exception { return iterQueue("deleteReference", DEFAULT_FLUSH_LIMIT, null, new QueueIterateAction(messageID) { @Override public boolean actMessage(Transaction tx, MessageReference ref) throws Exception { From 2284845e29a6eaae9ecf8f52e55dacec3896a88e Mon Sep 17 00:00:00 2001 From: Shivan Taher Date: Tue, 11 Aug 2026 10:21:36 +0200 Subject: [PATCH 3/3] ARTEMIS-6179 Drop removeMessage deadlock repro Removes testRemoveMessageWhilstPagingAndConsuming and ManagementRemoveThread, added earlier on this branch to demonstrate the deadlock before the fix. The reproduction is a probabilistic race rather than a deterministic test: it reliably caught the deadlock without the fix and passed cleanly with it, but relies on winning a narrow timing window rather than a guaranteed interleaving. Kept out of the permanent suite for that reason; it remains in branch history as evidence for the fix in the preceding two commits. --- .../ManagementWithPagingServerTest.java | 103 ------------------ 1 file changed, 103 deletions(-) diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ManagementWithPagingServerTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ManagementWithPagingServerTest.java index 875f754bddcb..09ddd9006bcb 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ManagementWithPagingServerTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ManagementWithPagingServerTest.java @@ -20,16 +20,12 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; import org.apache.activemq.artemis.api.core.RoutingType; import org.apache.activemq.artemis.json.JsonArray; import org.apache.activemq.artemis.json.JsonNumber; import org.apache.activemq.artemis.json.JsonObject; import org.apache.activemq.artemis.json.JsonValue; -import java.lang.management.ManagementFactory; -import java.lang.management.ThreadInfo; -import java.lang.management.ThreadMXBean; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.List; @@ -52,7 +48,6 @@ import org.apache.activemq.artemis.core.server.ActiveMQServers; import org.apache.activemq.artemis.core.settings.impl.AddressFullMessagePolicy; import org.apache.activemq.artemis.core.settings.impl.AddressSettings; -import org.apache.activemq.artemis.tests.util.Wait; import org.apache.activemq.artemis.utils.RandomUtil; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -327,72 +322,6 @@ public void testMoveMessageWhilstPagingAndConsuming() throws Exception { assertEquals(messages.length, console.copiedMessages); } - /** - * Reproduction for ARTEMIS-6179: QueueImpl#deleteReference() is still {@code synchronized} and calls - * iterQueue(), which locks depageLock -- while QueueImpl#depage() locks depageLock first and then enters a - * synchronized(this) block. Racing QueueControl#removeMessage() (-> deleteReference()) against depaging - * (triggered here by the ReceiverThread acking messages) can deadlock the two threads against each other. - * This mirrors testMoveMessageWhilstPagingAndConsuming(), which caught the equivalent bug for copyReference() - * (ARTEMIS-5376), replacing copyMessage with removeMessage. Detection uses the JVM's own deadlock detector - * (ThreadMXBean) instead of a fixed timeout, since a hang here is the failure itself. - */ - @Test - public void testRemoveMessageWhilstPagingAndConsuming() throws Exception { - final int messagesPerIteration = 2000; - final int maxIterations = 20; - final long iterationTimeoutMillis = 5000; - final long pollIntervalMillis = 20; - - SimpleString address = RandomUtil.randomUUIDSimpleString(); - SimpleString queue = RandomUtil.randomUUIDSimpleString(); - - session1.createQueue(QueueConfiguration.of(queue).setAddress(address)); - - QueueControl queueControl = createManagementControl(address, queue); - - ThreadMXBean threadMXBean = ManagementFactory.getThreadMXBean(); - - // A single remover is sufficient: QueueControlImpl#removeMessage() is serialized broker-wide through - // ActiveMQServerImpl's managementLock, so extra concurrent removers would just contend on that lock - // without increasing the odds of racing depage(). - ManagementRemoveThread remover = new ManagementRemoveThread(queueControl); - remover.setDaemon(true); - remover.start(); - - long[] deadlockedIds = null; - - for (int iteration = 0; iteration < maxIterations; iteration++) { - SenderThread sender = new SenderThread(address, messagesPerIteration, 0); - ReceiverThread receiver = new ReceiverThread(queue, messagesPerIteration, 0); - sender.setDaemon(true); - receiver.setDaemon(true); - - sender.start(); - receiver.start(); - - Wait.waitFor(() -> !receiver.isAlive() || threadMXBean.findDeadlockedThreads() != null, - iterationTimeoutMillis, pollIntervalMillis); - - deadlockedIds = threadMXBean.findDeadlockedThreads(); - if (deadlockedIds != null) { - break; - } - } - - remover.exit(); - remover.join(iterationTimeoutMillis); - assertNull(remover.getError()); - - if (deadlockedIds != null) { - StringBuilder sb = new StringBuilder("Deadlock detected between removeMessage() and depage():\n"); - for (long id : deadlockedIds) { - ThreadInfo info = threadMXBean.getThreadInfo(id, Integer.MAX_VALUE); - sb.append(info).append('\n'); - } - fail(sb.toString()); - } - } - @Override @BeforeEach public void setUp() throws Exception { @@ -578,36 +507,4 @@ public void exit() { stop = true; } } - - private class ManagementRemoveThread extends Thread { - - private QueueControl queueControl; - private volatile boolean stop = false; - private Exception error = null; - - private ManagementRemoveThread(QueueControl queueControl) { - this.queueControl = queueControl; - } - - @Override - public void run() { - try { - Random random = new Random(System.currentTimeMillis()); - while (!stop) { - long messageID = random.nextInt(5000); - queueControl.removeMessage(messageID); - } - } catch (Exception e) { - error = e; - } - } - - public Exception getError() { - return error; - } - - public void exit() { - stop = true; - } - } }