This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 06da787736 Fix automatic pong close race (#1041)
06da787736 is described below
commit 06da787736a39b686630a6a9ee44ebb54eb014c9
Author: Moritz <[email protected]>
AuthorDate: Fri Aug 14 12:10:13 2026 +0200
Fix automatic pong close race (#1041)
* Fix race between the automatic Pong response and session close
The check of Session.isOpen() and the subsequent sendPong() are not
atomic. If another thread starts the close process in between, the send
fails with an IllegalStateException that escapes on the thread that is
processing the incoming Ping.
Suppress that exception only if the close process has started by the
time the send fails. Any other failure is still reported.
* Add tests for the automatic Pong response during session close
Cover both branches: the send failure is swallowed once the close
process has started and it is propagated while the session is open.
* Changelog entry for the automatic Pong / session close race
---
java/org/apache/tomcat/websocket/WsFrameBase.java | 9 ++-
java/org/apache/tomcat/websocket/WsSession.java | 10 +++
test/org/apache/tomcat/websocket/TestWsFrame.java | 79 +++++++++++++++++++++++
webapps/docs/changelog.xml | 5 +-
4 files changed, 101 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/WsFrameBase.java
b/java/org/apache/tomcat/websocket/WsFrameBase.java
index b960c6cd3f..a3d3633df6 100644
--- a/java/org/apache/tomcat/websocket/WsFrameBase.java
+++ b/java/org/apache/tomcat/websocket/WsFrameBase.java
@@ -377,7 +377,14 @@ public abstract class WsFrameBase {
wsSession.onClose(new CloseReason(Util.getCloseCode(code),
reason));
} else if (opCode == Constants.OPCODE_PING) {
if (wsSession.isOpen()) {
- wsSession.getBasicRemote().sendPong(controlBufferBinary);
+ try {
+ wsSession.getBasicRemote().sendPong(controlBufferBinary);
+ } catch (IllegalStateException ise) {
+ // The close process may have started after isOpen() was
checked.
+ if (!wsSession.isClosing()) {
+ throw ise;
+ }
+ }
}
} else if (opCode == Constants.OPCODE_PONG) {
MessageHandler.Whole<PongMessage> mhPong =
wsSession.getPongMessageHandler();
diff --git a/java/org/apache/tomcat/websocket/WsSession.java
b/java/org/apache/tomcat/websocket/WsSession.java
index 4fc3ad22a4..3dc324b05f 100644
--- a/java/org/apache/tomcat/websocket/WsSession.java
+++ b/java/org/apache/tomcat/websocket/WsSession.java
@@ -458,6 +458,16 @@ public class WsSession implements Session {
}
+ /**
+ * Checks if the session close process has started.
+ *
+ * @return true if the session is closing or closed
+ */
+ boolean isClosing() {
+ return state.get() != State.OPEN;
+ }
+
+
/**
* Checks if the session is closed.
*
diff --git a/test/org/apache/tomcat/websocket/TestWsFrame.java
b/test/org/apache/tomcat/websocket/TestWsFrame.java
index c14386d68e..2096fbbf76 100644
--- a/test/org/apache/tomcat/websocket/TestWsFrame.java
+++ b/test/org/apache/tomcat/websocket/TestWsFrame.java
@@ -17,10 +17,17 @@
package org.apache.tomcat.websocket;
import java.io.IOException;
+import java.nio.ByteBuffer;
+import jakarta.websocket.RemoteEndpoint;
+
+import org.easymock.EasyMock;
import org.junit.Assert;
import org.junit.Test;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+
public class TestWsFrame {
@Test
@@ -58,4 +65,76 @@ public class TestWsFrame {
WsFrameBase.byteArrayToLong(new byte[] { 20, 127, -1, -1, -1,
-1, -1, -1, -1 }, 1, 8));
Assert.assertEquals(-1, WsFrameBase.byteArrayToLong(new byte[] { 20,
-1, -1, -1, -1, -1, -1, -1, -1 }, 1, 8));
}
+
+
+ @Test
+ public void testAutomaticPongAfterCloseStarted() throws Exception {
+ WsSession wsSession = EasyMock.createNiceMock(WsSession.class);
+ RemoteEndpoint.Basic basicRemote =
EasyMock.createMock(RemoteEndpoint.Basic.class);
+ EasyMock.expect(wsSession.isOpen()).andReturn(Boolean.TRUE);
+ EasyMock.expect(wsSession.getBasicRemote()).andReturn(basicRemote);
+ basicRemote.sendPong(EasyMock.anyObject(ByteBuffer.class));
+ EasyMock.expectLastCall().andThrow(new IllegalStateException());
+ EasyMock.expect(wsSession.isClosing()).andReturn(Boolean.TRUE);
+ EasyMock.replay(wsSession, basicRemote);
+
+ TestFrame frame = new TestFrame(wsSession);
+ frame.processPing();
+
+ EasyMock.verify(wsSession, basicRemote);
+ }
+
+
+ @Test
+ public void testAutomaticPongFailureWhileOpen() throws Exception {
+ WsSession wsSession = EasyMock.createNiceMock(WsSession.class);
+ RemoteEndpoint.Basic basicRemote =
EasyMock.createMock(RemoteEndpoint.Basic.class);
+ EasyMock.expect(wsSession.isOpen()).andReturn(Boolean.TRUE);
+ EasyMock.expect(wsSession.getBasicRemote()).andReturn(basicRemote);
+ basicRemote.sendPong(EasyMock.anyObject(ByteBuffer.class));
+ EasyMock.expectLastCall().andThrow(new IllegalStateException());
+ EasyMock.expect(wsSession.isClosing()).andReturn(Boolean.FALSE);
+ EasyMock.replay(wsSession, basicRemote);
+
+ TestFrame frame = new TestFrame(wsSession);
+ try {
+ frame.processPing();
+ Assert.fail();
+ } catch (IllegalStateException expected) {
+ // Expected.
+ }
+
+ EasyMock.verify(wsSession, basicRemote);
+ }
+
+
+ private static class TestFrame extends WsFrameBase {
+
+ TestFrame(WsSession wsSession) {
+ super(wsSession, null);
+ }
+
+ void processPing() throws IOException {
+ inputBuffer.clear();
+ inputBuffer.put((byte) 0x89);
+ inputBuffer.put((byte) 0x00);
+ inputBuffer.flip();
+ processInputBuffer();
+ }
+
+ @Override
+ protected boolean isMasked() {
+ return false;
+ }
+
+ @Override
+ protected Log getLog() {
+ return LogFactory.getLog(TestFrame.class);
+ }
+
+ @Override
+ protected void resumeProcessing() {
+ // NO-OP
+ }
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c777765e3f..bb64015f0e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -336,6 +336,10 @@
</subsection>
<subsection name="WebSocket">
<changelog>
+ <fix>
+ Fix an exception when an automatic Pong response races with the
+ closing of the WebSocket session. (moritzfl)
+ </fix>
<update>
Update Tomcat's WebSocket support to version 2.3 of the Jakarta
WebSocket API. (markt)
@@ -409,4 +413,3 @@
</section>
</body>
</document>
-
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
