This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new 8865b73f23 Fix automatic pong close race (#1041)
8865b73f23 is described below
commit 8865b73f23830aa98a274549a5e0a6c7098eb095
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.
---
java/org/apache/tomcat/websocket/WsFrameBase.java | 9 ++-
java/org/apache/tomcat/websocket/WsSession.java | 10 +++
test/org/apache/tomcat/websocket/TestWsFrame.java | 77 +++++++++++++++++++++++
webapps/docs/changelog.xml | 9 ++-
4 files changed, 103 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/tomcat/websocket/WsFrameBase.java
b/java/org/apache/tomcat/websocket/WsFrameBase.java
index b960c6cd3f..cb91c9cce0 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 e) {
+ // wsSession started to close or has fully closed while
pong was being prepared
+ if (!wsSession.isClosing()) {
+ throw e;
+ }
+ }
}
} 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 3908743eb2..ccc3b2b9c0 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..3b84c2aedf 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.junit.Assert;
import org.junit.Test;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.easymock.EasyMock;
+
public class TestWsFrame {
@Test
@@ -58,4 +65,74 @@ 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 {
+ doTestAutomaticPongFailure(new IllegalStateException(), true, true);
+ }
+
+
+ @Test
+ public void testAutomaticPongISEWhileOpen() throws Exception {
+ doTestAutomaticPongFailure(new IllegalStateException(), false, false);
+ }
+
+
+ private static void doTestAutomaticPongFailure(Exception failure, boolean
closing, boolean swallowed)
+ throws Exception {
+ WsSession wsSession = EasyMock.createNiceMock(WsSession.class);
+ RemoteEndpoint.Basic basicRemote =
EasyMock.createMock(RemoteEndpoint.Basic.class);
+
EasyMock.expect(Boolean.valueOf(wsSession.isOpen())).andReturn(Boolean.TRUE);
+ EasyMock.expect(wsSession.getBasicRemote()).andReturn(basicRemote);
+ basicRemote.sendPong(EasyMock.anyObject(ByteBuffer.class));
+ EasyMock.expectLastCall().andThrow(failure);
+
EasyMock.expect(Boolean.valueOf(wsSession.isClosing())).andStubReturn(Boolean.valueOf(closing));
+ EasyMock.replay(wsSession, basicRemote);
+
+ TestFrame frame = new TestFrame(wsSession);
+ if (swallowed) {
+ frame.processPing();
+ } else {
+ try {
+ frame.processPing();
+ Assert.fail();
+ } catch (Exception actual) {
+ Assert.assertSame(failure, actual);
+ }
+ }
+
+ 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 5643a741f7..eeee4747b5 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -115,6 +115,14 @@
</fix>
</changelog>
</subsection>
+ <subsection name="WebSocket">
+ <changelog>
+ <fix>
+ Fix an exception when an automatic Pong response races with the
+ closing of the WebSocket session. (moritzfl)
+ </fix>
+ </changelog>
+ </subsection>
</section>
<section name="Tomcat 10.1.58 (schultz)" rtext="not released">
<subsection name="Catalina">
@@ -8585,4 +8593,3 @@
</section>
</body>
</document>
-
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
