8129957: Deadlock in JNDI LDAP implementation when closing the LDAP context

Reviewed-by: vinnie

src/share/classes/com/sun/jndi/ldap/LdapClient.java

@@ -494,16 +494,14 @@ public final class LdapClient implements PooledConnection {
      */
     void processConnectionClosure() {
         // Notify listeners
-        synchronized (unsolicited) {
-            if (unsolicited.size() > 0) {
-                String msg;
-                if (conn != null) {
-                    msg = conn.host + ":" + conn.port + " connection closed";
-                } else {
-                    msg = "Connection closed";
-                }
-                notifyUnsolicited(new CommunicationException(msg));
+        if (unsolicited.size() > 0) {
+            String msg;
+            if (conn != null) {
+                msg = conn.host + ":" + conn.port + " connection closed";
+            } else {
+                msg = "Connection closed";
             }
+            notifyUnsolicited(new CommunicationException(msg));
         }
 
         // Remove from pool
@@ -1499,13 +1497,8 @@ public final class LdapClient implements PooledConnection {
         if (debug > 0) {
             System.err.println("LdapClient.removeUnsolicited" + ctx);
         }
-        synchronized (unsolicited) {
-            if (unsolicited.size() == 0) {
-                return;
-            }
             unsolicited.removeElement(ctx);
         }
-    }
 
     // NOTE: Cannot be synchronized because this is called asynchronously
     // by the reader thread in Connection. Instead, sync on 'unsolicited' Vector.
@@ -1513,30 +1506,35 @@ public final class LdapClient implements PooledConnection {
         if (debug > 0) {
             System.err.println("LdapClient.processUnsolicited");
         }
-        synchronized (unsolicited) {
-            try {
-                // Parse the response
-                LdapResult res = new LdapResult();
-
-                ber.parseSeq(null); // init seq
-                ber.parseInt();             // msg id; should be 0; ignored
-                if (ber.parseByte() != LDAP_REP_EXTENSION) {
-                    throw new IOException(
-                        "Unsolicited Notification must be an Extended Response");
-                }
-                ber.parseLength();
-                parseExtResponse(ber, res);
+        try {
+            // Parse the response
+            LdapResult res = new LdapResult();
+
+            ber.parseSeq(null); // init seq
+            ber.parseInt();             // msg id; should be 0; ignored
+            if (ber.parseByte() != LDAP_REP_EXTENSION) {
+                throw new IOException(
+                    "Unsolicited Notification must be an Extended Response");
+            }
+            ber.parseLength();
+            parseExtResponse(ber, res);
 
-                if (DISCONNECT_OID.equals(res.extensionId)) {
-                    // force closing of connection
-                    forceClose(pooled);
-                }
+            if (DISCONNECT_OID.equals(res.extensionId)) {
+                // force closing of connection
+                forceClose(pooled);
+            }
+
+            LdapCtx first = null;
+            UnsolicitedNotification notice = null;
 
+            synchronized (unsolicited) {
                 if (unsolicited.size() > 0) {
+                    first = unsolicited.elementAt(0);
+
                     // Create an UnsolicitedNotification using the parsed data
                     // Need a 'ctx' object because we want to use the context's
                     // list of provider control factories.
-                    UnsolicitedNotification notice = new UnsolicitedResponseImpl(
+                    notice = new UnsolicitedResponseImpl(
                         res.extensionId,
                         res.extensionValue,
                         res.referrals,
@@ -1544,42 +1542,45 @@ public final class LdapClient implements PooledConnection {
                         res.errorMessage,
                         res.matchedDN,
                         (res.resControls != null) ?
-                        unsolicited.elementAt(0).convertControls(res.resControls) :
+                        first.convertControls(res.resControls) :
                         null);
+                }
+            }
 
-                    // Fire UnsolicitedNotification events to listeners
-                    notifyUnsolicited(notice);
+            if (notice != null) {
+                // Fire UnsolicitedNotification events to listeners
+                notifyUnsolicited(notice);
 
-                    // If "disconnect" notification,
-                    // notify unsolicited listeners via NamingException
-                    if (DISCONNECT_OID.equals(res.extensionId)) {
-                        notifyUnsolicited(
-                            new CommunicationException("Connection closed"));
-                    }
+                // If "disconnect" notification,
+                // notify unsolicited listeners via NamingException
+                if (DISCONNECT_OID.equals(res.extensionId)) {
+                    notifyUnsolicited(
+                        new CommunicationException("Connection closed"));
                 }
-            } catch (IOException e) {
-                if (unsolicited.size() == 0)
-                    return;  // no one registered; ignore
-
-                NamingException ne = new CommunicationException(
-                    "Problem parsing unsolicited notification");
-                ne.setRootCause(e);
+            }
+        } catch (IOException e) {
+            NamingException ne = new CommunicationException(
+                "Problem parsing unsolicited notification");
+            ne.setRootCause(e);
 
-                notifyUnsolicited(ne);
+            notifyUnsolicited(ne);
 
-            } catch (NamingException e) {
-                notifyUnsolicited(e);
-            }
+        } catch (NamingException e) {
+            notifyUnsolicited(e);
         }
     }
 
 
     private void notifyUnsolicited(Object e) {
-        for (int i = 0; i < unsolicited.size(); i++) {
-            unsolicited.elementAt(i).fireUnsolicited(e);
+        Vector<LdapCtx> unsolicitedCopy;
+        synchronized (unsolicited) {
+            unsolicitedCopy = new Vector<>(unsolicited);
+            if (e instanceof NamingException) {
+                unsolicited.setSize(0);  // no more listeners after exception
+            }
         }
-        if (e instanceof NamingException) {
-            unsolicited.setSize(0);  // no more listeners after exception
+        for (int i = 0; i < unsolicitedCopy.size(); i++) {
+            unsolicitedCopy.elementAt(i).fireUnsolicited(e);
         }
     }