On Fri, 11 Jun 2021 14:42:14 GMT, Ivan Šipka <[email protected]> wrote:
>> @dfuch could you please review, thank you.
>
> Ivan Šipka has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8263364: refactor
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 45:
> 43: static class XServer extends Thread implements AutoCloseable {
> 44:
> 45: volatile ServerSocket serverSocket;
could be final instead of volatile
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 112:
> 110: @Override
> 111: public void close() throws Exception {
> 112: clientSocket.close();
you should really guard against null here (using a local variable to capture
clientSocket) since clientSocket could be transiently null until the run()
method is called.
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 120:
> 118: public static void main (String[] args) throws Exception {
> 119:
> 120: final Integer port = 61234;
Why suddenly use a known port? That's a recipe for intermittent test failures.
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 147:
> 145: e.printStackTrace();
> 146: }
> 147:
Why catch exception? That will hide test failures. Let exceptions percolate out
of main instead.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4472
