On Friday 24 August 2018 08:52 PM, Chris Hegarty wrote:
Hi Vyom,
On 24/08/18 11:35, vyom tewari wrote:
Hi All,
Please review this simple fix below
webrev: http://cr.openjdk.java.net/~vtewari/8205330/webrev0.0/index.html
bugid: https://bugs.openjdk.java.net/browse/JDK-8205330
This fix will resolve the race in LdapClient where we are explicitly
making "null" to LdapClient.conn.
Sorry, I don't know this code all that well, but I think
that more explanation will be needed before this code
can be reviewed.
Hi Chris, let me try to explain issue little bit.
The issue is a if ldap connection has already been established and then
the LDAP directory server sends an (unsolicited) "Notice of
Disconnection", the client's processing of this LDAP message can race
with an application thread calling new InitialDirContext() to
authenticate user credentials (i.e.bind) and throw NPE.
I did some analysis and found out that when server send an (unsolicited)
"Notice of Disconnection", LdapClient.forceClose() will be called in
LdapClient.processUnsolicited() which is called asynchronously by the
reader thread in Connection, this means 'LdapClient.conn' may set to
null anytime after it received "Notice of Disconnection".
The LdapClient and the Connection seem to be loosely
coupled. I think part of this is to allow the LdapClient
to be GC'ed and finalized separately to the Connection
( that can be reused ). Not setting `conn` to null could
have a negative impact on this loose coupling. I also
note that the synchronization is implemented poorly in
the LdapClient, `conn` is operated on both from within
synchronized blocks and from unsynchronized blocks (
which I think is the reason you see the unexpected
null ).
I agree, not setting 'conn' to null will definitely have some impact.
I check the LdapClient and it looks to me it is intentional(i may be
wrong) that 'conn' is operated on both from within synchronize blocks
and from unsynchronize block.
LdapClient, java doc says that connection(conn) take care of it's own sync.
##################################
access from outside LdapClient must sync;
* conn - no sync; Connection takes care of its own sync
* unsolicited - sync Vector; multiple operations sync'ed
##################################
Please have a look and do let me know your thought on the above.
Thanks,
Vyom
-Chris.
