# HG changeset patch # User robm # Date 1481915737 0 # Node ID d477072ef0e4f509366d23095826b30da28dbe74 # Parent b59eb84f0b0fdc343909b9751006c1261bd70706 8169465: Deadlock in com.sun.jndi.ldap.pool.Connections Reviewed-by: dfuchs, vtewari diff -r b59eb84f0b0f -r d477072ef0e4 jdk/src/java.naming/share/classes/com/sun/jndi/ldap/pool/Connections.java --- a/jdk/src/java.naming/share/classes/com/sun/jndi/ldap/pool/Connections.java Fri Dec 16 09:42:33 2016 -0800 +++ b/jdk/src/java.naming/share/classes/com/sun/jndi/ldap/pool/Connections.java Fri Dec 16 19:15:37 2016 +0000 @@ -27,7 +27,6 @@ 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 @@ * @param threshold an entry idle since this time has expired. * @return true if no more connections in list */ - synchronized boolean expire(long threshold) { - Iterator iter = conns.iterator(); - ConnectionDesc entry; - while (iter.hasNext()) { - entry = iter.next(); + boolean expire(long threshold) { + List clonedConns; + synchronized(this) { + clonedConns = new ArrayList<>(conns); + } + List expired = new ArrayList<>(); + + for (ConnectionDesc entry : clonedConns) { + d("expire(): ", entry); if (entry.expire(threshold)) { - d("expire(): removing ", entry); - td("Expired ", entry); - - iter.remove(); // remove from pool - - // Don't need to call notify() because we're - // removing only idle connections. If there were - // idle connections, then there should be no waiters. + 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 -r b59eb84f0b0f -r d477072ef0e4 jdk/src/java.naming/share/classes/com/sun/jndi/ldap/pool/Pool.java --- a/jdk/src/java.naming/share/classes/com/sun/jndi/ldap/pool/Pool.java Fri Dec 16 09:42:33 2016 -0800 +++ b/jdk/src/java.naming/share/classes/com/sun/jndi/ldap/pool/Pool.java Fri Dec 16 19:15:37 2016 +0000 @@ -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 @@ * and removed. */ public void expire(long threshold) { + Collection copy; synchronized (map) { - Iterator 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 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(); }