This is an automated email from the ASF dual-hosted git repository.

markt 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 473b6a70be Review after flagging by Coverity Scan
473b6a70be is described below

commit 473b6a70bead3fb2f6931efb4bf8eab9d539da5f
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Sep 4 16:54:06 2025 +0100

    Review after flagging by Coverity Scan
    
    The SocketWrapper lock is unnecessary. The calling thread will always
    hold the lock.
    sequence needs to be volatile as it will be accessed by different
    threads but not concurrently - and always with the SocketWrapper lock
    held.
---
 .../apache/coyote/http2/Http2UpgradeHandler.java    | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java 
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index f7036e4363..93847c1b5a 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1865,7 +1865,7 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
         // 10 seconds
         protected final long pingIntervalNano = 10000000000L;
 
-        protected int sequence = 0;
+        protected volatile int sequence = 0;
         protected long lastPingNanoTime = Long.MIN_VALUE;
 
         protected Queue<PingRecord> inflightPings = new 
ConcurrentLinkedQueue<>();
@@ -1886,18 +1886,13 @@ class Http2UpgradeHandler extends AbstractStream 
implements InternalHttpUpgradeH
             if (force || now - lastPingNanoTime > pingIntervalNano) {
                 lastPingNanoTime = now;
                 byte[] payload = new byte[8];
-                socketWrapper.getLock().lock();
-                try {
-                    int sentSequence = ++sequence;
-                    PingRecord pingRecord = new PingRecord(sentSequence, now);
-                    inflightPings.add(pingRecord);
-                    ByteUtil.set31Bits(payload, 4, sentSequence);
-                    socketWrapper.write(true, PING, 0, PING.length);
-                    socketWrapper.write(true, payload, 0, payload.length);
-                    socketWrapper.flush(true);
-                } finally {
-                    socketWrapper.getLock().unlock();
-                }
+                int sentSequence = ++sequence;
+                PingRecord pingRecord = new PingRecord(sentSequence, now);
+                inflightPings.add(pingRecord);
+                ByteUtil.set31Bits(payload, 4, sentSequence);
+                socketWrapper.write(true, PING, 0, PING.length);
+                socketWrapper.write(true, payload, 0, payload.length);
+                socketWrapper.flush(true);
             }
         }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to