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

[Rob McKenna]

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