RFR:8205330 InitialDirContext ctor sometimes throws NPE if the server has sent a disconnection

Daniel Fuchs daniel.fuchs at oracle.com
Tue Sep 4 14:43:19 UTC 2018


Hi Vyom,

IIUC, the issue only happens when connections (clients?) to the
server are pooled. I assume the server may decide to
close what it thinks is an idle connection at any time.

So what we see here is actually a race between an open
connection being retrieved from the pool, and the server
deciding to close that connection.
The connection/client is retrieved from the pool, then
unfortunately closed by the server after having been selected
for the next operation.

The question for me would be "what is the expected behavior
if the server decides to close an idle connection?"
I would expect that new InitialDirContext() should have some
way of dealing with that with retrying?
If so will leaving the 'conn' field assigned ensure that
the retry happens, or will new InitialDirContext() fail
with some other exception because the connection has
been closed? Or does the code simply assume that asynchronous
closing of the connection by the server can only happen while
it's sitting idle in the pool?

I wonder if some other mechanism could be used to reduce
and hopefully eliminate the time window in which the race
could appear. For instance taking the connection out of
the pool before closing it, instead of closing it before
taking it out of the pool, etc...

best regards,

-- daniel

On 04/09/2018 15:04, vyom tewari wrote:
> 
> 
> 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.
> 



More information about the core-libs-dev mailing list