Hi Xuelei,
I am not an official reviewer, but I have a couple of comments.
1. line 149: would it be better to check this condition in a loop?
2. Using try-with-resources blocks might simplify doServerSide() a
little bit (no need to call close() on sockets, and a couple of "try"
blocks might be probably removed)
3. Minor: it might be better to define timeout values as constants with
meaningful names.
4. Minor: lines 268-273 seem to work, but I am wondering if it would be
better to use File.separator instead of "/" (someone may run the test on
Windows, but without Cygwin).
5. SSLSocketSample() constructor, startServer() and startClient() methods:
Recently we've had a couple of test issue where no exception was printed
on client/server sides. It happened because exceptions were caught and
stored, but then tests waited infinitely for another thread to finish.
Then, jtreg just killed the tests. As a result, no exception was printed
out. It might be better to simplify this code to print out an exception
right after it's caught, and then throw a runtime exception if
serverException or clientException are not null. Probably there is a
side effect that logs may be a little messy sometimes because of lack of
synchronization, but maybe something like synchronized(System.out) may help.
6. Minor: it might be better to make class constants uppercase
http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-135099.html#367
Artem
On 07/10/2016 10:16 PM, Xuelei Fan wrote:
There is a nice catch of the timeout miss-match during the handling of
serverIsReady in doClientSide(). Here is the updated webrev:
http://cr.openjdk.java.net/~xuelei/8161106/webrev.01/
Thanks,
Xuelei
On 7/11/2016 11:44 AM, Xuelei Fan wrote:
Hi,
Please review this enhancement of SSLSocket test template:
http://cr.openjdk.java.net/~xuelei/8161106/webrev.00/
There are some known issues with the current SSLSocket test template
(test/javax/net/ssl/templates/SSLSocketTemplate.java) in the automation
testing environment:
1. the client or server can be blocked if the peer run into problems, as
may result in intermittent timeout failure.
2. the server accepted connection may be not linked to the expected
client. For example, some other test case may try to use and connect to
a free server socket port, unfortunately this port can be actually
opened by the server of SSLSocketTemplate because there is no
synchronization about the free socket port between test cases. It's OK
to run the test singly, but may result in weird intermittent failure in
the automation testing environment.
The new test template in this update considers the noises above.
Thanks,
Xuelei
