# HG changeset patch # User robm # Date 1442527188 -3600 # Node ID a76c2d877a16430d005e73ae16d2529ecd216f5f # Parent 8dfeae0ff33294ecaf0f2860b42e83eff2a94342 8129957: Deadlock in JNDI LDAP implementation when closing the LDAP context Reviewed-by: vinnie diff -r 8dfeae0ff332 -r a76c2d877a16 jdk/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java --- a/jdk/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java Thu Sep 17 13:43:06 2015 -0700 +++ b/jdk/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java Thu Sep 17 22:59:48 2015 +0100 @@ -494,16 +494,14 @@ */ 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 @@ 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 @@ if (debug > 0) { System.err.println("LdapClient.processUnsolicited"); } - synchronized (unsolicited) { - try { - // Parse the response - LdapResult res = new LdapResult(); + 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); - 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 @@ 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 "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 + } + + if (notice != null) { + // Fire UnsolicitedNotification events to listeners + notifyUnsolicited(notice); - NamingException ne = new CommunicationException( - "Problem parsing unsolicited notification"); - ne.setRootCause(e); + // If "disconnect" notification, + // notify unsolicited listeners via NamingException + if (DISCONNECT_OID.equals(res.extensionId)) { + notifyUnsolicited( + new CommunicationException("Connection closed")); + } + } + } 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 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); } }