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

remm 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 cf2c5f9762 Fix bad use of some remove while iterating
cf2c5f9762 is described below

commit cf2c5f9762883eccbfc926c5d858a99cecc77c03
Author: remm <r...@apache.org>
AuthorDate: Mon Sep 25 14:19:00 2023 +0200

    Fix bad use of some remove while iterating
    
    FairBlockingQueue was always handling it properly though.
    Also add a few missing syncs.
    Fix a couple of possible rare NPEs.
    Found by coverity.
---
 .../org/apache/tomcat/jdbc/pool/ConnectionPool.java  | 20 +++++++++++++-------
 .../apache/tomcat/jdbc/pool/FairBlockingQueue.java   |  8 +++++++-
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git 
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
 
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
index 5e8adbbd03..30492c3971 100644
--- 
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
+++ 
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
@@ -1092,7 +1092,7 @@ public class ConnectionPool {
                     long time = con.getTimestamp();
                     long now = System.currentTimeMillis();
                     if (shouldAbandon() && (now - time) > 
con.getAbandonTimeout()) {
-                        busy.remove(con);
+                        locked.remove();
                         abandon(con);
                         setToNull = true;
                     } else if (sto > 0 && (now - time) > (sto * 1000L)) {
@@ -1143,7 +1143,7 @@ public class ConnectionPool {
                     if (shouldReleaseIdle(now, con, time)) {
                         releasedIdleCount.incrementAndGet();
                         release(con);
-                        idle.remove(con);
+                        unlocked.remove();
                         setToNull = true;
                     } else {
                         //do nothing
@@ -1207,7 +1207,7 @@ public class ConnectionPool {
                     }
                     if (release) {
                         releasedIdleCount.incrementAndGet();
-                        idle.remove(con);
+                        unlocked.remove();
                         release(con);
                     }
                 } finally {
@@ -1462,7 +1462,9 @@ public class ConnectionPool {
                 if (configured.compareAndSet(false, true)) {
                     try {
                         pc = borrowConnection(System.currentTimeMillis(),pc, 
null, null);
-                        result = ConnectionPool.this.setupConnection(pc);
+                        if (pc != null) {
+                            result = ConnectionPool.this.setupConnection(pc);
+                        }
                     } catch (SQLException x) {
                         cause = x;
                     } finally {
@@ -1504,7 +1506,9 @@ public class ConnectionPool {
         public void run() {
             try {
                 Connection con = get(); //complete this future
-                con.close(); //return to the pool
+                if (con != null) {
+                    con.close(); //return to the pool
+                }
             }catch (ExecutionException ex) {
                 //we can ignore this
             }catch (Exception x) {
@@ -1560,7 +1564,8 @@ public class ConnectionPool {
         }
     }
 
-    public static Set<TimerTask> getPoolCleaners() {
+    // Testing use only
+    public static synchronized Set<TimerTask> getPoolCleaners() {
         return Collections.<TimerTask>unmodifiableSet(cleaners);
     }
 
@@ -1568,7 +1573,8 @@ public class ConnectionPool {
         return poolVersion.get();
     }
 
-    public static Timer getPoolTimer() {
+    // Testing use only
+    public static synchronized Timer getPoolTimer() {
         return poolCleanTimer;
     }
 
diff --git 
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java
 
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java
index 80d90c2b69..6b23c1abe3 100644
--- 
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java
+++ 
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/FairBlockingQueue.java
@@ -232,7 +232,13 @@ public class FairBlockingQueue<E> implements 
BlockingQueue<E> {
 
     @Override
     public int size() {
-        return items.size();
+        final ReentrantLock lock = this.lock;
+        lock.lock();
+        try {
+            return items.size();
+        } finally {
+            lock.unlock();
+        }
     }
 
     @Override


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

Reply via email to