RFR: 8146213, 8133093: java/nio/channels/ServerSocketChannel/AdaptServerSocket.java failed intermittently with Connection refused and Accept did not time out

[Brian Burkhalter]

Hi Hamlin,

On Jan 13, 2016, at 7:32 PM, Hamlin Li <huaming.li at oracle.com> wrote:

> Would you please help to review the fix for below bugs?
> https://bugs.openjdk.java.net/browse/JDK-8146213, Test java/nio/channels/ServerSocketChannel/AdaptServerSocket.java failed intermittently with Connection refused,
> and https://bugs.openjdk.java.net/browse/JDK-8133093, Test java/nio/channels/ServerSocketChannel/AdaptServerSocket.java failed with Accept did not time out.
> 
> webrev : http://cr.openjdk.java.net/~mli/8146213/webrev.00/

This looks OK to me aside from the following two points:

1. At line 2, 2013 should be changed to 2016

The general approach for copyright years is discussed in http://mail.openjdk.java.net/pipermail/openjfx-dev/2015-March/016757.html. I could not locate a specification for this in the OpenJDK guidelines which does not of course indicate it is not there.

2. The changeset is missing a comment

See the section “Formatting a Changeset Comment” in http://openjdk.java.net/guide/producingChangeset.html. In a case such as this where there is more than one issue there should be one bugid line per issue. Also as you do not yet have commit rights to JDK 9, the contributed-by line should be included with your e-mail address.

I don’t know whether you’ve done it already, but you should run jcheck (http://openjdk.java.net/projects/code-tools/jcheck/) on the patch and correct any problems. These commonly devolve to either trailing whitespace in a file or an invalid reviewer attribution. The whitespace may be corrected using <FOREST_ROOT>/make/scripts/normalizer.pl. Note that if you are using the mercurial Mq extension, you will need to run “hg qref” after normalizing any files.

Lastly, the labels “noreg-self” and “test-only” should I believe be added to both JBS issues.

Note that I am not a JDK Reviewer so I cannot formally approve the review request.

Thanks,

Brian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20160114/5fc6e9e7/attachment.html>