From 20e6ca3601d2748b049d918a5f8c50e5f4191207 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 16 Sep 2019 13:14:06 +0100 Subject: [PATCH] Fix race condition in sendBlockingMessage Previously, tests in ConcurrentWebSocketSessionDecoratorTests that use the BlockingSession would fail intermittently. This appears to have been due to a race condition in sendBlockingMessage where the call to getSentMessageLatch() that stores a latch in nextMessageLatch on the main thread may happen after the call to sendMessage that counts down the latch if it is non-null occurs on the executor's thread. This commit updates sendBlockingMessage to call getSentMessageLatch() (and therefore store the latch) before it sumbmits the task to the executor. This ensures that the latch will be available when the exeuctor's thread attempts to retrieve and decrement it. BlockingSession's AtomicReference fields have also been made final to eliminate the possibility of any visibility problems across threads. Closes gh-23642 --- .../ConcurrentWebSocketSessionDecoratorTests.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/handler/ConcurrentWebSocketSessionDecoratorTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/handler/ConcurrentWebSocketSessionDecoratorTests.java index 0f06aba3a4..937e1d992a 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/handler/ConcurrentWebSocketSessionDecoratorTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/handler/ConcurrentWebSocketSessionDecoratorTests.java @@ -203,6 +203,8 @@ public class ConcurrentWebSocketSessionDecoratorTests { } private void sendBlockingMessage(ConcurrentWebSocketSessionDecorator session) throws InterruptedException { + BlockingSession delegate = (BlockingSession) session.getDelegate(); + CountDownLatch sentMessageLatch = delegate.getSentMessageLatch(); Executors.newSingleThreadExecutor().submit(() -> { TextMessage message = new TextMessage("slow message"); try { @@ -212,17 +214,16 @@ public class ConcurrentWebSocketSessionDecoratorTests { e.printStackTrace(); } }); - BlockingSession delegate = (BlockingSession) session.getDelegate(); - assertThat(delegate.getSentMessageLatch().await(5, TimeUnit.SECONDS)).isTrue(); + assertThat(sentMessageLatch.await(5, TimeUnit.SECONDS)).isTrue(); } private static class BlockingSession extends TestWebSocketSession { - private AtomicReference nextMessageLatch = new AtomicReference<>(); + private final AtomicReference nextMessageLatch = new AtomicReference<>(); - private AtomicReference releaseLatch = new AtomicReference<>(); + private final AtomicReference releaseLatch = new AtomicReference<>(); public CountDownLatch getSentMessageLatch() {