Skip to content

Commit 5851cdc

Browse files
committed
Refine invocation of checkSessions
It makes more sense to call this from afterConnectionEstablished as it relates to the creation of new sessions. See gh-32195
1 parent 2e833d9 commit 5851cdc

File tree

2 files changed

+17
-10
lines changed

2 files changed

+17
-10
lines changed

spring-websocket/src/main/java/org/springframework/web/socket/messaging/SubProtocolWebSocketHandler.java

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -317,6 +317,8 @@ public void afterConnectionEstablished(WebSocketSession session) throws Exceptio
317317
return;
318318
}
319319

320+
checkSessions();
321+
320322
this.stats.incrementSessionCount(session);
321323
session = decorateSession(session);
322324
this.sessions.put(session.getId(), new WebSocketSessionHolder(session));
@@ -337,7 +339,6 @@ public void handleMessage(WebSocketSession session, WebSocketMessage<?> message)
337339
if (holder != null) {
338340
holder.setHasHandledMessages();
339341
}
340-
checkSessions();
341342
}
342343

343344
/**
@@ -475,16 +476,17 @@ private String resolveSessionId(Message<?> message) {
475476
}
476477

477478
/**
478-
* When a session is connected through a higher-level protocol it has a chance
479-
* to use heartbeat management to shut down sessions that are too slow to send
480-
* or receive messages. However, after a WebSocketSession is established and
481-
* before the higher level protocol is fully connected there is a possibility for
482-
* sessions to hang. This method checks and closes any sessions that have been
483-
* connected for more than 60 seconds without having received a single message.
479+
* A higher-level protocol can use heartbeats to detect sessions that need to
480+
* be cleaned up. However, if a WebSocket session is established, but messages
481+
* can't flow (e.g. due to a proxy issue), then the higher level protocol is
482+
* never successfully negotiated, and without heartbeats, sessions can hang.
483+
* The method checks for sessions that have not received any messages 60
484+
* seconds after the WebSocket session was established, and closes them.
484485
*/
485486
private void checkSessions() {
486487
long currentTime = System.currentTimeMillis();
487-
if (!isRunning() || (currentTime - this.lastSessionCheckTime < getTimeToFirstMessage())) {
488+
long timeSinceLastCheck = currentTime - this.lastSessionCheckTime;
489+
if (!isRunning() || timeSinceLastCheck < getTimeToFirstMessage() / 2) {
488490
return;
489491
}
490492

spring-websocket/src/test/java/org/springframework/web/socket/messaging/SubProtocolWebSocketHandlerTests.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,13 +182,18 @@ public void checkSession() throws Exception {
182182
this.webSocketHandler.start();
183183
this.webSocketHandler.handleMessage(session1, new TextMessage("foo"));
184184

185+
TestWebSocketSession session3 = new TestWebSocketSession("id3");
186+
session3.setOpen(true);
187+
session3.setAcceptedProtocol("v12.stomp");
188+
this.webSocketHandler.afterConnectionEstablished(session1);
189+
185190
assertThat(session1.isOpen()).isTrue();
186191
assertThat(session1.getCloseStatus()).isNull();
187192

188193
assertThat(session2.isOpen()).isFalse();
189194
assertThat(session2.getCloseStatus()).isEqualTo(CloseStatus.SESSION_NOT_RELIABLE);
190195

191-
assertThat(handlerAccessor.getPropertyValue("lastSessionCheckTime")).as("lastSessionCheckTime not updated").isNotEqualTo(sixtyOneSecondsAgo);
196+
assertThat(handlerAccessor.getPropertyValue("lastSessionCheckTime")).isNotEqualTo(sixtyOneSecondsAgo);
192197
}
193198

194199
}

0 commit comments

Comments
 (0)