This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/11.0.x by this push:
new c4785d2e1d Fix automatic pong close race (#1041)
c4785d2e1d is described below
commit c4785d2e1d000f3f13f5f234ad70c5e5e441366b
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 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..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 0cac92408a..07a6401f3a 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 11.0.25 (markt)" rtext="release in progress">
<subsection name="Catalina">
@@ -6420,4 +6428,3 @@
</section>
</body>
</document>
-
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
