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
