[12] RFR 8210695: Create test to cover JDK-8205330 InitialDirContext ctor sometimes throws NPE if the server has sent a disconnection

[Chris Yin]

Thank you, Daniel

Regards,
Chris

> On 14 Sep 2018, at 6:19 PM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> Hi Chris,
> 
> > http://cr.openjdk.java.net/~xyin/8210695/webrev.01/
> 
> Looks good to me.
> 
> > Yep, fixed as you suggested. My original though is that the stack trace maybe too verbose if most of runs hit NamingException, leave it to stderr may make the log more readable, but yes, the exception is expected and not considered as error as you commented. Anyway, we could adjust it later if necessary.
> 
> An alternative then would be to print an additional message on
> stderr - something like:
> 
>  89                 } catch (NamingException ne) {
>  90                     System.out.println("(" + count + "/" + REPEAT_COUNT
>  91                             + ") It's ok to get NamingException: " + ne);
>  92                     // for debug
>                         System.err.println("Caught expected exception: " + ne);
>  93                     ne.printStackTrace();
> 
> If you decide to go this way instead then no need for an additional
> review...
> 
> best regards,
> 
> -- daniel
> 
> 
> 
> On 14/09/2018 04:42, Chris Yin wrote:
>> Hi, Daniel
>> Thank you for the reviewing and comments, inline and update webrev as below, thanks
>> http://cr.openjdk.java.net/~xyin/8210695/webrev.01/
>>> On 13 Sep 2018, at 5:43 PM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
>>> 
>>> Hi Chris,
>>> 
>>> Thanks a lot for writing this test!
>>> 
>>>  60         serverSocket = new ServerSocket(0);
>>>  69         env.put(Context.PROVIDER_URL, String.format("ldap://localhost:%d/",
>>> 
>>> For robustness of the test I would suggest using
>>> InetAddress.getLoopbackAddress() explicitly, to
>>> avoid having the dummy server listen on all interfaces.
>>> Listening on all interfaces has proved to be a source of
>>> intermittent failures in the past, possibly due
>>> to different port reuse strategies at OS level.
>> Sure, fixed, thanks
>>> 
>>>  88                     System.out.println("(" + count + "/" + REPEAT_COUNT
>>>  89                             + ") It's ok to get NamingException: " + ne);
>>>  90                     // for debug
>>>  91                     ne.printStackTrace();
>>> 
>>> I think it would be better in that case to print the stactrace on
>>> stdout (i.e. use ne.printStackTrace(System.out)), since that
>>> exception is expected and not considered as an error.
>> Yep, fixed as you suggested. My original though is that the stack trace maybe too verbose if most of runs hit NamingException, leave it to stderr may make the log more readable, but yes, the exception is expected and not considered as error as you commented. Anyway, we could adjust it later if necessary.
>> Regards,
>> Chris
>>> 
>>> best regards,
>>> 
>>> -- daniel
>>> 
>>> 
>>> On 13/09/2018 07:59, Chris Yin wrote:
>>>> Please have a review for below new added test to cover JDK-8205330, thanks
>>>> This test used dummy ldap server to simulate the scenario to send “Notice of Disconnection” after binding, it will repeat InitialDirContext() for 1000 times (normally the NPE bug should be hit less than 100 times run, but just in case), test failed with NPE if without JDK-8205330 fix change and passed after the fix.
>>>> I put this test under test/jdk/com/sun/jndi/ldap/ since the bug root cause is from com.sun.jndi.ldap.LdapClient, feel free to let me know if it should be moved to other suitable folder.
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8210695
>>>> webrev: http://cr.openjdk.java.net/~xyin/8210695/webrev.00/
>>>> Regards,
>>>> Chris
>>> 
>