RFR: 8263364: sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java wedged in getInputStream [v2]

Daniel Fuchs dfuchs at openjdk.java.net
Fri Jun 11 14:53:50 UTC 2021


On Fri, 11 Jun 2021 14:42:14 GMT, Ivan Šipka <isipka at openjdk.org> 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


More information about the net-dev mailing list