Your proposed fix looks fine Rob. Thanks.
> On 14 Sep 2015, at 16:25, Rob McKenna <rob.mcke...@oracle.com> wrote: > > Hi folks, > > So on further investigation it looks like we could get away with reducing the > amount of locking in LdapClient. Here is a proposed fix followed by a > description: > > http://cr.openjdk.java.net/~robm/8129957/webrev.02/ > > processConnectionClosure(): > - Remove the synchronization from processConnectionClosure and handle it > further down in notifyUnsolicited > > removeUnsolicited(): > - Remove the synchronized block in removeUnsolicited as its redundant. > Vectors are synchronized already. > > processUnsolicited() > - Remove the initial synchronized block in processUnsolicited and limit it to > the area around the unsolicited size check / notice creation. (again, due to > the notifyUnsolicited changes) > - Remove the redundant unsolicited.size check from the end of > processUnsolicited > > notifyUnsolicited(): > - Synchronize on the unsolicited vector in order to create a copy of that > vector and empty it if e is a NamingException. > - Outside the notifyUnsolicited synchronize block, loop through the copy of > unsolicited and call fireUnsolicited on each element. > - The main potential problem with this fix would be if an LdapCtx became > unsolicited before we got to it in the for loop. However since both > LdapCtx.fireUnsolicited and LdapCtx.removeUnsolicited sync on eventSupport > and LdapCtx.fireUnsolicited double checks to make sure it is still > unsolicited, that should be fine. > > -Rob > > > On 10/08/15 14:06, Rob McKenna wrote: >> Hi folks, >> >> We have a hang between LdapClient / Ctx due to the fact that >> Connection.cleanup() calls LdapClient.processConnectionClosure which >> locks the unsolicited vector and in turn calls LdapCtx.fireUnsolicited >> which locks the eventSupport object. Unfortunately when an >> LdapCtx.close() occurs at the same time, its removeUnsolicited method >> locks the eventSupport object first and then attempts to call >> LdapClient.removeUnsolicited which attempts to lock the unsolicited vector. >> >> So: >> >> thread 1: >> >> Connection.cleanup -> >> LdapClient.processConnectionClosure (LOCK VECTOR) -> >> LdapCtx.fireUnsolicited (LOCK EVENTSUPPORT) >> >> (LdapClient is looping through LdapCtx objects in the unsolicited vector) >> >> thread 2: >> >> LdapCtx.close (LOCK LDAPCTX) -> >> LdapCtx.removeUnsolicited (LOCK EVENTSUPPORT) -> >> LdapClient.removeUnsolicited (LOCK VECTOR) >> >> (A single LdapCtx removes itself from its LdapClient unsolicited list) >> >> >> My proposed solution is to have both threads lock the LdapClient before >> locking either the unsolicited vector or the eventSupport object. >> >> Webrev at: http://cr.openjdk.java.net/~robm/8129957/webrev.01/ >> >> -Rob
