Code Review Request: 7044870: TEST_BUG: java/nio/channels/DatagramChannel/SelectWhenRefused.java failed on SUSE Linux 10

chris hegarty chris.hegarty at oracle.com
Tue Aug 23 13:36:07 PDT 2011


I'm not sure if it is the output of hg diff or your editor, but there 
appears to be stylistic issues.

   - removal of four space indentation of main method and several 
following declarations
   - open curly brace for finally/catch should be on same line
   - finally clause should start on the same line as the closing curly brace

-Chris.

There seems to be

On 23/08/2011 19:38, Kurchi Hazra wrote:
> hg diff for the updated code:
>
> -bash-3.00$ hg diff
> diff --git 
> a/test/java/nio/channels/DatagramChannel/SelectWhenRefused.java 
> b/test/java/nio/channels/DatagramChannel/SelectWhenRefused.java
> --- a/test/java/nio/channels/DatagramChannel/SelectWhenRefused.java
> +++ b/test/java/nio/channels/DatagramChannel/SelectWhenRefused.java
> @@ -34,17 +34,18 @@ import java.io.IOException;
>
>  public class SelectWhenRefused {
>
> -    public static void main(String[] args) throws IOException {
> -        DatagramChannel dc = DatagramChannel.open().bind(new 
> InetSocketAddress(0));
> -        int port = dc.socket().getLocalPort();
> -        dc.close();
> +  public static void main(String[] args) throws IOException {
> +     DatagramChannel dc1 = DatagramChannel.open().bind(new 
> InetSocketAddress(0));
> +     int port = dc1.socket().getLocalPort();
> +     DatagramChannel dc = DatagramChannel.open().bind(new 
> InetSocketAddress(0));
> +     dc1.close();
>
> -        // datagram sent to this address should be refused
> -        SocketAddress refuser = new 
> InetSocketAddress(InetAddress.getLocalHost(), port);
> +     // datagram sent to this address should be refused
>
> -        dc = DatagramChannel.open().bind(new InetSocketAddress(0));
> -        Selector sel = Selector.open();
> -        try {
> +     SocketAddress refuser = new 
> InetSocketAddress(InetAddress.getLocalHost(), port);
> +     Selector sel = Selector.open();
> +
> +    try {
>              dc.configureBlocking(false);
>              dc.register(sel, SelectionKey.OP_READ);
>
> @@ -52,12 +53,14 @@ public class SelectWhenRefused {
>              sendDatagram(dc, refuser);
>              int n = sel.select(2000);
>              if (n > 0) {
> +                sel.selectedKeys().clear();
> +                DatagramSocket sock=new DatagramSocket(port);
> +                sock.close();
>                  throw new RuntimeException("Unexpected wakeup");
> -            }
> -
> -            /* Test 2: connected so ICMP port unreachable may be 
> received */
> -            dc.connect(refuser);
> -            try {
> +                }
> +                           /* Test 2: connected so ICMP port 
> unreachable may be received */
> +             try {
> +                dc.connect(refuser);
>                  sendDatagram(dc, refuser);
>                  n = sel.select(2000);
>                  if (n > 0) {
> @@ -80,11 +83,18 @@ public class SelectWhenRefused {
>                  throw new RuntimeException("Unexpected wakeup after 
> disconnect");
>              }
>
> -        } finally {
> +        }
> +        catch (BindException e)
> +        {
> +                //Do nothing, someone else has bound to this port
> +        }
> +         finally {
>              sel.close();
>              dc.close();
>          }
> +
>      }
> +
>
>      static void sendDatagram(DatagramChannel dc, SocketAddress remote)
>          throws IOException
>
>
> On 8/23/2011 11:04 AM, Kurchi Hazra wrote:
>>
>>
>> On 8/23/2011 8:08 AM, Chris Hegarty wrote:
>>> Hi Kurchi,
>>>
>>> I see your change is checking if another service is using the 
>>> expected refuser port, and then ignoring any data since the test 
>>> cannot be successfully run. I think this is good, but it may also be 
>>> useful to defer closing of dc1 until after creation of dc. That way 
>>> this issue shouldn't happen as often since dc will not reuse the 
>>> refusers port. Make sense? Also, doesn't this impact on the rest of 
>>> the test too?
>>>
>>> Would just catch BindException be sufficient? That's what you're 
>>> expecting, right, "Address already in use"?
>>
>>
>>
>> - I agree, it should be a BindException that I catch. Also, I will 
>> incorporate deferring of closing of dc1 until after dc is allocated a 
>> port.
>>
>> -Kurchi
>>
>>
>>>
>>> -Chris.
>>>
>>> On 23/08/2011 01:29, Kurchi Hazra wrote:
>>>> Hi,
>>>>
>>>>
>>>> Test 1 in java/nio/channels/DatagramChannel/SelectWhenRefused.java was
>>>> failing on some Linux kernels. This is because the test assumes 
>>>> that the
>>>> kernel will not bind a socket to a port that has been closed 
>>>> immediately.
>>>> However, some linux kernels (for example, 2.6.16.60-0.62.1-smp ) are
>>>> precisely doing this and the test throws a RunTime Exception.
>>>>
>>>> The code has been changed to not to try binding the same socket 
>>>> that has
>>>> been immediately closed. In addition, we make sure that a port is not
>>>> bound to a service already (in between the port being closed before 
>>>> and
>>>> connected to later in the code) by checking before throwing the 
>>>> Exception.
>>>>
>>>>
>>>> Submitting hg diff since I do not have an openjdk account:
>>>>
>>>> -bash-3.00$ hg diff
>>>> diff --git
>>>> a/test/java/nio/channels/DatagramChannel/SelectWhenRefused.java
>>>> b/test/java/nio/channels/DatagramChannel/SelectWhenRefused.java
>>>> --- a/test/java/nio/channels/DatagramChannel/SelectWhenRefused.java
>>>> +++ b/test/java/nio/channels/DatagramChannel/SelectWhenRefused.java
>>>> @@ -35,14 +35,14 @@ public class SelectWhenRefused {
>>>> public class SelectWhenRefused {
>>>>
>>>> public static void main(String[] args) throws IOException {
>>>> - DatagramChannel dc = DatagramChannel.open().bind(new
>>>> InetSocketAddress(0));
>>>> - int port = dc.socket().getLocalPort();
>>>> - dc.close();
>>>> + DatagramChannel dc1 = DatagramChannel.open().bind(new
>>>> InetSocketAddress(0));
>>>> + int port = dc1.socket().getLocalPort();
>>>> + dc1.close();
>>>>
>>>> // datagram sent to this address should be refused
>>>> SocketAddress refuser = new
>>>> InetSocketAddress(InetAddress.getLocalHost(), port);
>>>>
>>>> - dc = DatagramChannel.open().bind(new InetSocketAddress(0));
>>>> + DatagramChannel dc = DatagramChannel.open().bind(new
>>>> InetSocketAddress(0));
>>>> Selector sel = Selector.open();
>>>> try {
>>>> dc.configureBlocking(false);
>>>> @@ -52,7 +52,15 @@ public class SelectWhenRefused {
>>>> sendDatagram(dc, refuser);
>>>> int n = sel.select(2000);
>>>> if (n > 0) {
>>>> + try {
>>>> + DatagramSocket dsBindCheck=new DatagramSocket(port);
>>>> + dsBindCheck.close();
>>>> throw new RuntimeException("Unexpected wakeup");
>>>> + }
>>>> + catch(SocketException e)
>>>> + {
>>>> + //Do nothing,some other test may have used the port
>>>> + }
>>>> }
>>>>
>>>> /* Test 2: connected so ICMP port unreachable may be received */
>>>>
>>>>
>>>> Thanks,
>>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20110823/c9d2cbf8/attachment.html 


More information about the nio-dev mailing list