RFR 8164533: [TEST_BUG] sun/security/ssl/SSLSocketImpl/CloseSocket.java failed with "Error while cleaning up threads after test"
Xuelei Fan
xuelei.fan at oracle.com
Thu Aug 25 01:10:41 UTC 2016
On 8/25/2016 3:55 AM, Artem Smotrakov wrote:
> Hi Svetlana,
>
> Thank you for cleaning up this test. I have a couple of comments (mostly
> about the original test).
>
> 1. I see that the test tries to connect to a server three times, but the
> server accept only first connection, and then it stops. So test cases
> #2-3 fail just because the connection was refused. The original test
> behaves like this. This looks like a bug to me. What do you think?
> Should the server have a loop of three iterations?
>
I think it's the purpose to test behavior of the close server socket.
> 2. Here is server's code:
>
> 95 @Override
> 96 public void run() {
> 97 try (Socket s = serverSocket.accept()) {
> 98 System.out.println("Server accepted connection");
> 99 // wait a bit before closing the socket to give
> 100 // the client time to send its hello message
> 101 Thread.currentThread().sleep(100);
> 102 s.close();
> 103 System.out.println("Server closed socket, done.");
> 104 } catch (Exception e) {
> 105 throw new RuntimeException("Problem in test
> execution", e);
> 106 }
> 107 }
>
> Not sure if it is a good assumption to expect that ClientHello is
> received in 100 milliseconds. It might read first data, and then close
> the socket. It also doesn't seem to be necessary to call close() there.
>
It is not expected to perform handshaking for this test. Get
connection, and immediately close the socket before handshaking, and
then see what happens in client side (connect/read/write).
I have the same concern that 100 MS may be an issue. Please consider to
make an improvement.
BTW, please run the test in othervm mode.
Xuelei
> Otherwise, the webrev looks good to me, but please note that I am not an
> official reviewer. You may want to fix the issues above, or we can just
> file a new bug.
>
> Artem
>
> On 08/24/2016 11:21 AM, Svetlana Nikandrova wrote:
>> Hello,
>>
>> please review this test bug fix. Test failed because of staled threads
>> left after execution.
>> Added try-with-resources statements to make sure test closes it's
>> resources. Also as test is overall quite old-fashioned I've done some
>> refactoring (hope now it looks better).
>>
>> JBS:
>> https://bugs.openjdk.java.net/browse/JDK-8164533
>> Webrev:
>> http://cr.openjdk.java.net/~snikandrova/8164533/webrev.00/
>> <http://cr.openjdk.java.net/%7Esnikandrova/8164533/webrev.00/>
>>
>> Thank you,
>> Svetlana
>
More information about the security-dev
mailing list