RFR: 8293696: java/nio/channels/DatagramChannel/SelectWhenRefused.java fails with "Unexpected wakeup" [v3]

[Daniel Fuchs]

On Tue, 1 Nov 2022 11:49:43 GMT, Darragh Clarke <duke at openjdk.org> wrote:

>> Added logging to SelectWhenRefused for an intermittent failure caused by unexpected wakeups, this includes trying to receive data if there is any available to identify where it is coming from.
>> 
>> I also set the test to run in othervm mode which should reduce the chances of this failure happening in the first place.
>
> Darragh Clarke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   implemented feedback

Changes requested by dfuchs (Reviewer).

test/jdk/java/nio/channels/DatagramChannel/SelectWhenRefused.java line 58:

> 56:             dc.register(sel, SelectionKey.OP_READ);
> 57: 
> 58:             for (int i = 0; i < 3; i++) {

I suggest to define a `static int MAX_TRIES = 3;` and use that here:

for (int i = 0; i < MAX_TRIES; i++) {

test/jdk/java/nio/channels/DatagramChannel/SelectWhenRefused.java line 73:

> 71:                     throw new RuntimeException("Unexpected wakeup");
> 72:                 }
> 73:             }

This should be:

                    if (ignoreStrayWakeup) {
                        if (i < MAX_TRIES - 1) continue;
                    }
                    // BindException will be thrown if another service is using 
                    // our expected refuser port, cannot run just exit.
                    DatagramChannel.open().bind(refuser).close();
                    throw new RuntimeException("Unexpected wakeup");
                } else {
                    break; 
                }

We don't want to silently finish and exit the loop if the last attempt failed. And we don't want to retry if n == 0.

test/jdk/java/nio/channels/DatagramChannel/SelectWhenRefused.java line 74:

> 72:                 }
> 73:             }
> 74:             for (int i = 0; i < 3; i++) {

The loop should start after connecting.
It should be:

                /* Test 2: connected so ICMP port unreachable may be received */
                dc.connect(refuser);
                try {
                     for (int i = 0;  i < MAX_TRIES; i++) {
                         ...
                     }
                 } finally {
                      ...

test/jdk/java/nio/channels/DatagramChannel/SelectWhenRefused.java line 97:

> 95:                         } catch (PortUnreachableException pue) {
> 96:                             // expected
> 97:                         }

This should be:

                            throw new RuntimeException("PortUnreachableException not raised");
                        } catch (PortUnreachableException pue) {
                            // expected
                        }
                    }
                    break;

If we get `PortUnreachableException` we have what we expected and we can exit the loop. Same if n == 0;

test/jdk/java/nio/channels/DatagramChannel/SelectWhenRefused.java line 103:

> 101:                 }
> 102:             }
> 103:             for (int i = 0; i < 3; i++) {

Use MAX_TRIES here too

test/jdk/java/nio/channels/DatagramChannel/SelectWhenRefused.java line 106:

> 104:                 /* Test 3: not connected so ICMP port unreachable should not be received */
> 105:                 sendDatagram(dc, refuser);
> 106:                 int n = sel.select(2000);

Same here:

                    if (ignoreStrayWakeup) {
                        if (i < MAX_TRIES - 1) continue;
                    }

test/jdk/java/nio/channels/DatagramChannel/SelectWhenRefused.java line 128:

> 126:         throws IOException
> 127:     {
> 128:         ByteBuffer bb = ByteBuffer.wrap("Greetings from SelectWhenRefused!".getBytes());

You could put the message in a static string constant, that would avoid repeating it.

-------------

PR: https://git.openjdk.org/jdk/pull/10851