This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 7c7fe25718 Fix automatic pong close race (#1041)
7c7fe25718 is described below
commit 7c7fe257188d708dc8ccdbdcac14a4b6e350b5aa
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 6eda7188a9..600456f9b6 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 bcf0e123b9..389b60c4de 100644
--- a/java/org/apache/tomcat/websocket/WsSession.java
+++ b/java/org/apache/tomcat/websocket/WsSession.java
@@ -552,6 +552,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 31bc6e521f..f6dc27902e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -132,6 +132,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 9.0.121 (remm)" rtext="release in progress">
<subsection name="Catalina">
@@ -19043,4 +19051,3 @@
</section>
</body>
</document>
-
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
