JDK 9 RFR of JDK-8146745: Problem list SessionCacheSizeTests.java

[Xuelei Fan]

On 1/11/2016 10:50 AM, joe darcy wrote:
> Hi Xuelei,
> 
> I'm not a concurrency expert, but I don't think it is proper to only
> synchronize on the writing of a data structure -- the reading should be
> synchronized too.
> 
Yes.  The fix considered the reading side too.  The reading of the
serverPorts will not happen unless the write side finished.  The
behavior is controlled by the "serverReady" variable.  Therefore, it is
OK to synchronize the write side of serverPorts in the test case.

The problem is mainly about the updated of the counter variable
createdPorts. Both the reading and writing sides are synchronized in the
same block.

> Have you reproduced the failure with the code and seen the failure go
> away with the new code? Is there a hypothesis on why the failure started
> to happen recently when the test doesn't seem to have been modified?
> 
It used to failure intermittent.  But I cannot find the root cause in
the past.  So a socket accept timeout was added in order to find the
underlying issues of the intermittent failure.  The socket accept
timeout would increase the frequency of the failure.  And it did help to
expose the root cause of the failure.

> As a code review comment, I recommend in the doServerSide method using a
> try-with-resources statement to manage the sslServerSocket variable.
> 
It's a better style.  I will make the update.

> For a larger refactoring, is an array with a separate createdPorts count
> the best data structure to use here?
> 
It could be more clear, I think.  But need too much refactoring, and may
introduce new synchronization requirement.  I will like to try this
simple update at first, and see what happens.

Thanks,
Xuelei

> Thanks,
> 
> -Joe
> 
> On 1/10/2016 4:11 PM, Xuelei Fan wrote:
>> Looks fine to me.
>>
>> BTW, the fix for JDK-8146387 is asking for code review.
>>
>> http://mail.openjdk.java.net/pipermail/security-dev/2016-January/013301.html
>>
>>
>> Thanks,
>> Xuelei
>>
>> On 1/11/2016 4:54 AM, joe darcy wrote:
>>> Hello,
>>>
>>> The test
>>>
>>> javax/net/ssl/SSLSession/SessionCacheSizeTests.java
>>>
>>> has been failing intermittently with high-frequency on Solaris and
>>> Windows of late. The test should be problem listed until the underlying
>>> problem is fixed (JDK-8146387).
>>>
>>> Patch to do this below.
>>>
>>> Thanks,
>>>
>>> -Joe
>>>
>>> diff -r aa9fd2797b82 test/ProblemList.txt
>>> --- a/test/ProblemList.txt    Sun Jan 10 11:09:31 2016 -0800
>>> +++ b/test/ProblemList.txt    Sun Jan 10 12:53:16 2016 -0800
>>> @@ -1,6 +1,6 @@
>>>  
>>> ###########################################################################
>>>
>>>
>>>   #
>>> -# Copyright (c) 2009, 2015, Oracle and/or its affiliates. All rights
>>> reserved.
>>> +# Copyright (c) 2009, 2016, Oracle and/or its affiliates. All rights
>>> reserved.
>>>   # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>   #
>>>   # This code is free software; you can redistribute it and/or modify it
>>> @@ -299,6 +299,9 @@
>>>   # 8074580
>>>   sun/security/pkcs11/rsa/TestKeyPairGenerator.java generic-all
>>>
>>> +# 8146387
>>> +javax/net/ssl/SSLSession/SessionCacheSizeTests.java
>>> windows-all,solaris-all
>>> +
>>>  
>>> ############################################################################
>>>
>>>
>>>
>>>   # jdk_sound
>>> diff -r aa9fd2797b82
>>> test/javax/net/ssl/SSLSession/SessionCacheSizeTests.java
>>> --- a/test/javax/net/ssl/SSLSession/SessionCacheSizeTests.java Sun Jan
>>> 10 11:09:31 2016 -0800
>>> +++ b/test/javax/net/ssl/SSLSession/SessionCacheSizeTests.java Sun Jan
>>> 10 12:53:16 2016 -0800
>>> @@ -31,6 +31,7 @@
>>>    * @bug 4366807
>>>    * @summary Need new APIs to get/set session timeout and session cache
>>> size.
>>>    * @run main/othervm SessionCacheSizeTests
>>> + * @key intermittent
>>>    */
>>>
>>>   import java.io.*;
>>>
>>>
>