Code Review Request JDK-8146387, Test SSLSession/SessionCacheSizeTests socket accept timed out

Xuelei Fan xuelei.fan at oracle.com
Mon Jan 11 09:13:27 UTC 2016


On 1/11/2016 5:09 PM, Xuelei Fan wrote:
> On 1/11/2016 4:10 PM, Wang Weijun wrote:
>> Can you make sure every read to serverPorts is after serverReady = true?
> Not actually.  The serverPorts is accessed before the serverReady is true.
> 
>> I hope so but cannot confirm it, especially in the constructor (on line
>> 342 etc). Or shouldn't you always start the server at port 0 and then
>> read it into serverPorts?
> Yes. That's the current behavior to use port 0 and than set the used
> port into serverPorts.
> 
>> You want to start a server on the same port on and on?
>>
> Not actually.  Different port are used for different server socket.  The
> logic looks like:
>    // define four slots for the ports
>    int serverPorts[] = new int[]{0, 0, 0, 0};
> 
>    // for each slot, create a server socket,
>    // and assign the actually used port for each slot.
>    for each slots run a thread {
>       SSLServerSocket sslServerSocket = ...
>       serverPorts[nextPort] = sslServerSocket.getLocalPort();
>                   ^^^^^^^^
>    }
> 
>    // use the actual port for each slot in client side
>    if (server ready) {
>       connect to the server socket (host:port)
>    }
> 
> The concurrent access to serverPorts is not the underlying problem.  The
> problem is which slot should be assigned the right port value.
> 
> Before this fix, the following code is prolematic:
> 
>     serverPorts[createdPorts++] = sslServerSocket.getLocalPort();
> 
> For example,  two threads access createdPorts, which is 0.
> createdPorts++ set createdPorts to 1.
And the #0 slot get updated, #1 slot not.  That's also a problem.

> Two server ports are generated,
> but only count once.  The serverReady would never set to true:
> 
>     if (createdPorts == serverPorts.length) {
>         serverReady = true;
>     }
> 
> 
> This fix is trying to increase the createdPorts value properly.  It is
> not actually need to synchronize serverPorts.
> 
> Thanks,
> Xuelei
> 
>> Thanks
>> Max
>>
>>> On Jan 9, 2016, at 7:06 PM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
>>>
>>> Hi,
>>>
>>> Please review a test update for JDK-8146387:
>>>
>>>   http://cr.openjdk.java.net/~xuelei/8146387/webrev.00/
>>>
>>> In the test case, javax/net/ssl/SSLSession/SessionCacheSizeTests.java,
>>> an integer (createdPorts) is used to count the server sockets.  Every
>>> server socket is created in a new thread.  The access and update to
>>> createdPorts should be synchronized for multiple threading safe.
>>>
>>> Thanks,
>>> Xuelei
>>
> 




More information about the security-dev mailing list