From 58dbc8f707314e09ec349c0be2809c238795c314 Mon Sep 17 00:00:00 2001
From: robm <unknown>
Date: Mon, 9 Jan 2017 18:33:02 +0000
Subject: [PATCH] 8169465: Deadlock in com.sun.jndi.ldap.pool.Connections
 Reviewed-by: dfuchs, vtewari

---
 .../com/sun/jndi/ldap/pool/Connections.java   | 34 +++++++++++--------
 .../classes/com/sun/jndi/ldap/pool/Pool.java  | 26 +++++++++-----
 2 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/src/share/classes/com/sun/jndi/ldap/pool/Connections.java b/src/share/classes/com/sun/jndi/ldap/pool/Connections.java
index 2df134595..4f05e8499 100644
--- a/src/share/classes/com/sun/jndi/ldap/pool/Connections.java
+++ b/src/share/classes/com/sun/jndi/ldap/pool/Connections.java
@@ -27,7 +27,6 @@ package com.sun.jndi.ldap.pool;
 
 import java.util.ArrayList; // JDK 1.2
 import java.util.List;
-import java.util.Iterator;
 
 import java.lang.ref.Reference;
 import java.lang.ref.SoftReference;
@@ -290,23 +289,28 @@ final class Connections implements PoolCallback {
      * @param threshold an entry idle since this time has expired.
      * @return true if no more connections in list
      */
-    synchronized boolean expire(long threshold) {
-        Iterator<ConnectionDesc> iter = conns.iterator();
-        ConnectionDesc entry;
-        while (iter.hasNext()) {
-            entry = iter.next();
-            if (entry.expire(threshold)) {
-                d("expire(): removing ", entry);
-                td("Expired ", entry);
-
-                iter.remove();  // remove from pool
+    boolean expire(long threshold) {
+        List<ConnectionDesc> clonedConns;
+        synchronized(this) {
+            clonedConns = new ArrayList<>(conns);
+        }
+        List<ConnectionDesc> expired = new ArrayList<>();
 
-                // Don't need to call notify() because we're
-                // removing only idle connections. If there were
-                // idle connections, then there should be no waiters.
+        for (ConnectionDesc entry : clonedConns) {
+            d("expire(): ", entry);
+            if (entry.expire(threshold)) {
+                expired.add(entry);
+                td("expire(): Expired ", entry);
             }
         }
-        return conns.isEmpty();  // whether whole list has 'expired'
+
+        synchronized (this) {
+            conns.removeAll(expired);
+            // Don't need to call notify() because we're
+            // removing only idle connections. If there were
+            // idle connections, then there should be no waiters.
+            return conns.isEmpty();  // whether whole list has 'expired'
+        }
     }
 
     /**
diff --git a/src/share/classes/com/sun/jndi/ldap/pool/Pool.java b/src/share/classes/com/sun/jndi/ldap/pool/Pool.java
index 29c4d3290..45b71ed24 100644
--- a/src/share/classes/com/sun/jndi/ldap/pool/Pool.java
+++ b/src/share/classes/com/sun/jndi/ldap/pool/Pool.java
@@ -25,11 +25,11 @@
 
 package com.sun.jndi.ldap.pool;
 
+import java.util.ArrayList;
 import java.util.Map;
 import java.util.WeakHashMap;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Iterator;
 import java.util.LinkedList;
 
 import java.io.PrintStream;
@@ -166,17 +166,25 @@ final public class Pool {
      *          and removed.
      */
     public void expire(long threshold) {
+        Collection<ConnectionsRef> copy;
         synchronized (map) {
-            Iterator<ConnectionsRef> iter = map.values().iterator();
-            Connections conns;
-            while (iter.hasNext()) {
-                conns = iter.next().getConnections();
-                if (conns.expire(threshold)) {
-                    d("expire(): removing ", conns);
-                    iter.remove();
-                }
+            copy = new ArrayList<>(map.values());
+        }
+
+        ArrayList<ConnectionsRef> removed = new ArrayList<>();
+        Connections conns;
+        for (ConnectionsRef ref : copy) {
+            conns = ref.getConnections();
+            if (conns.expire(threshold)) {
+                d("expire(): removing ", conns);
+                removed.add(ref);
             }
         }
+
+        synchronized (map) {
+            map.values().removeAll(removed);
+        }
+
         expungeStaleConnections();
     }
 
-- 
GitLab