diff --git a/src/main/java/network/crypta/clients/fcp/FCPConnectionOutputHandler.java b/src/main/java/network/crypta/clients/fcp/FCPConnectionOutputHandler.java index d91446b945..ed3f05b892 100644 --- a/src/main/java/network/crypta/clients/fcp/FCPConnectionOutputHandler.java +++ b/src/main/java/network/crypta/clients/fcp/FCPConnectionOutputHandler.java @@ -160,9 +160,11 @@ private void flushOutput(OutputStream os) throws IOException { } private QueueAction nextQueueAction(boolean flushedSinceLastSend) throws InterruptedException { - synchronized (outQueue) { - while (true) { - boolean closed = handler.isClosed(); + while (true) { + // Read the handler state before taking the queue monitor to keep lock ordering consistent + // with input-side code paths that already hold the handler lock and then inspect outQueue. + boolean closed = handler.isClosed(); + synchronized (outQueue) { if (!outQueue.isEmpty()) { return QueueAction.message(outQueue.removeFirst()); } diff --git a/src/test/java/network/crypta/clients/fcp/FCPConnectionOutputHandlerTest.java b/src/test/java/network/crypta/clients/fcp/FCPConnectionOutputHandlerTest.java index 5976ecb471..75cc6be141 100644 --- a/src/test/java/network/crypta/clients/fcp/FCPConnectionOutputHandlerTest.java +++ b/src/test/java/network/crypta/clients/fcp/FCPConnectionOutputHandlerTest.java @@ -18,10 +18,12 @@ import org.mockito.junit.jupiter.MockitoExtension; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -91,6 +93,27 @@ void run_whenHandlerAlreadyClosed_flushesAndClosesStream() throws IOException { assertTrue(outputHandler.closedOutputQueue); } + @Test + void run_whenCheckingClosedState_doesNotHoldQueueMonitor() throws IOException { + when(handler.getSocket()).thenReturn(socket); + when(socket.getOutputStream()).thenReturn(outputStream); + doAnswer( + invocation -> { + assertFalse(Thread.holdsLock(outputHandler.outQueue)); + return true; + }) + .when(handler) + .isClosed(); + + outputHandler.run(); + + verify(outputStream, atLeastOnce()).flush(); + verify(outputStream).close(); + verify(handler).close(); + verify(handler).closedOutput(); + assertTrue(outputHandler.closedOutputQueue); + } + @Test void run_whenMessageQueued_sendsMessageBeforeClosing() throws IOException { FCPMessage message = mock(FCPMessage.class);