8249786: java/net/httpclient/websocket/PendingPingTextClose.java fails very infrequently

TransportImpl is modified to make sure the CLOSED state is recorded before the channel is closed. The tests are modified to enable their retry mechanism on windows, similar to what was done previously for macOS.

Reviewed-by: prappo, chegar

src/java.net.http/share/classes/jdk/internal/net/http/websocket/TransportImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -308,13 +308,15 @@ public class TransportImpl implements Transport {
                 try {
                     channel.shutdownOutput();
                 } finally {
+                    writeState.set(CLOSED);
                     if (inputClosed) {
                         channel.close();
                     }
                 }
             }
         }
-        writeState.set(CLOSED);
+        ChannelState s = writeState.get();
+        assert s == CLOSED : s;
         sendScheduler.runOrSchedule();
     }
 
@@ -335,6 +337,8 @@ public class TransportImpl implements Transport {
                     channel.shutdownInput();
                 } finally {
                     if (outputClosed) {
+                        ChannelState s = writeState.get();
+                        assert s == CLOSED : s;
                         channel.close();
                     }
                 }

test/jdk/java/net/httpclient/websocket/PendingBinaryPingClose.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -79,8 +79,9 @@ public class PendingBinaryPingClose extends PendingOperations {
             assertFails(ISE, webSocket.sendPong(ByteBuffer.allocate(125)));
             cfClose = webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "ok");
             assertHangs(cfClose);
+            assertNotDone(cfBinary);
             return null;
-        }, () -> cfBinary.isDone() ? true : false);
+        }, () -> cfBinary.isDone());
         webSocket.abort();
         assertFails(IOE, cfBinary);
         assertFails(IOE, cfPing);

test/jdk/java/net/httpclient/websocket/PendingBinaryPongClose.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -79,8 +79,9 @@ public class PendingBinaryPongClose extends PendingOperations {
             assertFails(ISE, webSocket.sendPong(ByteBuffer.allocate(125)));
             cfClose = webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "ok");
             assertHangs(cfClose);
+            assertNotDone(cfBinary);
             return null;
-        }, () -> cfBinary.isDone() ? true : false);
+        }, () -> cfBinary.isDone());
         webSocket.abort();
         assertFails(IOE, cfBinary);
         assertFails(IOE, cfPong);

test/jdk/java/net/httpclient/websocket/PendingOperations.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -27,6 +27,7 @@ import org.testng.annotations.DataProvider;
 import java.io.IOException;
 import java.net.http.WebSocket;
 import java.util.concurrent.Callable;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionStage;
 import java.util.function.BooleanSupplier;
 
@@ -46,6 +47,7 @@ public class PendingOperations {
 
     @AfterTest
     public void cleanup() {
+        System.err.println("cleanup: Closing server");
         server.close();
         webSocket.abort();
     }
@@ -61,6 +63,10 @@ public class PendingOperations {
         Support.assertCompletesExceptionally(clazz, stage);
     }
 
+    static void assertNotDone(CompletableFuture<?> future) {
+        Support.assertNotDone(future);
+    }
+
     @DataProvider(name = "booleans")
     public Object[][] booleans() {
         return new Object[][]{{Boolean.TRUE}, {Boolean.FALSE}};
@@ -69,6 +75,9 @@ public class PendingOperations {
     static boolean isMacOS() {
         return System.getProperty("os.name").contains("OS X");
     }
+    static boolean isWindows() {
+        return System.getProperty("os.name").toLowerCase().startsWith("win");
+    }
 
     private static final int ITERATIONS = 3;
 
@@ -84,7 +93,12 @@ public class PendingOperations {
                 callable.call();
                 break;
             } catch (AssertionError e) {
-                if (isMacOS() && repeatCondition.getAsBoolean()) {
+                var isMac = isMacOS();
+                var isWindows = isWindows();
+                var repeat = repeatCondition.getAsBoolean();
+                System.out.printf("repeatable: isMac=%s, isWindows=%s, repeat=%s, iterations=%d%n",
+                                  isMac, isWindows, repeat, iterations);
+                if ((isMac || isWindows) && repeat) {
                     // ## This is loathsome, but necessary because of observed
                     // ## automagic socket buffer resizing on recent macOS platforms
                     continue;

test/jdk/java/net/httpclient/websocket/PendingPingBinaryClose.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -77,8 +77,9 @@ public class PendingPingBinaryClose extends PendingOperations {
             assertHangs(cfBinary);
             cfClose = webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "ok");
             assertHangs(cfClose);
+            assertNotDone(cfPing);
             return null;
-        }, () -> cfPing.isDone() ? true : false);
+        }, () -> cfPing.isDone());
         webSocket.abort();
         assertFails(IOE, cfPing);
         assertFails(IOE, cfBinary);

test/jdk/java/net/httpclient/websocket/PendingPingTextClose.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -44,44 +44,61 @@ import static java.net.http.HttpClient.newBuilder;
 
 public class PendingPingTextClose extends PendingOperations {
 
+    static boolean debug = false; // avoid too verbose output
     CompletableFuture<WebSocket> cfText;
     CompletableFuture<WebSocket> cfPing;
     CompletableFuture<WebSocket> cfClose;
 
     @Test(dataProvider = "booleans")
     public void pendingPingTextClose(boolean last) throws Exception {
-        repeatable( () -> {
+        try {
-            server = Support.notReadingServer();
+            repeatable(() -> {
-            server.open();
+                server = Support.notReadingServer();
-            webSocket = newBuilder().proxy(NO_PROXY).build().newWebSocketBuilder()
+                server.open();
-                    .buildAsync(server.getURI(), new WebSocket.Listener() { })
+                webSocket = newBuilder().proxy(NO_PROXY).build().newWebSocketBuilder()
-                    .join();
+                        .buildAsync(server.getURI(), new WebSocket.Listener() {
-            ByteBuffer data = ByteBuffer.allocate(125);
+                        })
-            for (int i = 0; ; i++) {  // fill up the send buffer
+                        .join();
-                long start = System.currentTimeMillis();
+                ByteBuffer data = ByteBuffer.allocate(125);
-                System.out.printf("begin cycle #%s at %s%n", i, start);
+                boolean done = false;
-                cfPing = webSocket.sendPing(data);
+                for (int i = 0; ; i++) {  // fill up the send buffer
-                try {
+                    long start = System.currentTimeMillis();
-                    cfPing.get(MAX_WAIT_SEC, TimeUnit.SECONDS);
+                    if (debug) System.out.printf("begin cycle #%s at %s%n", i, start);
-                    data.clear();
+                    cfPing = webSocket.sendPing(data);
-                } catch (TimeoutException e) {
+                    try {
-                    break;
+                        cfPing.get(MAX_WAIT_SEC, TimeUnit.SECONDS);
-                } finally {
+                        data.clear();
-                    long stop = System.currentTimeMillis();
+                    } catch (TimeoutException e) {
-                    System.out.printf("end cycle #%s at %s (%s ms)%n", i, stop, stop - start);
+                        done = true;
+                        System.out.printf("Got expected timeout after %d iterations%n", i);
+                        break;
+                    } finally {
+                        long stop = System.currentTimeMillis();
+                        if (debug || done || (stop - start) > (MAX_WAIT_SEC * 1000L)/2L)
+                            System.out.printf("end cycle #%s at %s (%s ms)%n", i, stop, stop - start);
+                    }
                 }
-            }
+                assertFails(ISE, webSocket.sendPing(ByteBuffer.allocate(125)));
-            assertFails(ISE, webSocket.sendPing(ByteBuffer.allocate(125)));
+                assertFails(ISE, webSocket.sendPong(ByteBuffer.allocate(125)));
-            assertFails(ISE, webSocket.sendPong(ByteBuffer.allocate(125)));
+                System.out.println("asserting that sendText hangs");
-            cfText = webSocket.sendText("hello", last);
+                cfText = webSocket.sendText("hello", last);
-            assertHangs(cfText);
+                assertHangs(cfText);
-            cfClose = webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "ok");
+                System.out.println("asserting that sendClose hangs");
-            assertHangs(cfClose);
+                cfClose = webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "ok");
-            return null;
+                assertHangs(cfClose);
-        }, () -> cfPing.isDone() ? true : false);
+                System.out.println("asserting that cfPing is not completed");
-        webSocket.abort();
+                assertNotDone(cfPing);
-        assertFails(IOE, cfPing);
+                System.out.println("finishing");
-        assertFails(IOE, cfText);
+                return null;
-        assertFails(IOE, cfClose);
+            }, () -> cfPing.isDone()); // can't use method ref: cfPing not initialized
+            webSocket.abort();
+            assertFails(IOE, cfPing);
+            assertFails(IOE, cfText);
+            assertFails(IOE, cfClose);
+        } catch (Throwable t) {
+            System.err.printf("pendingPingTextClose(%s) failed: %s%n", last, t);
+            t.printStackTrace();
+            throw t;
+        }
     }
 }

test/jdk/java/net/httpclient/websocket/PendingPongBinaryClose.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -77,8 +77,9 @@ public class PendingPongBinaryClose extends PendingOperations {
             assertHangs(cfBinary);
             cfClose = webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "ok");
             assertHangs(cfClose);
+            assertNotDone(cfPong);
             return null;
-        }, () -> cfPong.isDone() ? true : false);
+        }, () -> cfPong.isDone());
         webSocket.abort();
         assertFails(IOE, cfPong);
         assertFails(IOE, cfBinary);

test/jdk/java/net/httpclient/websocket/PendingPongTextClose.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -79,8 +79,9 @@ public class PendingPongTextClose extends PendingOperations {
             assertHangs(cfText);
             cfClose = webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "ok");
             assertHangs(cfClose);
+            assertNotDone(cfPong);
             return  null;
-        }, () -> cfPong.isDone() ? true : false);
+        }, () -> cfPong.isDone());
         webSocket.abort();
         assertFails(IOE, cfPong);
         assertFails(IOE, cfText);

test/jdk/java/net/httpclient/websocket/PendingTextPingClose.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -80,8 +80,9 @@ public class PendingTextPingClose extends PendingOperations {
             assertFails(ISE, webSocket.sendPong(ByteBuffer.allocate(125)));
             cfClose = webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "ok");
             assertHangs(cfClose);
+            assertNotDone(cfText);
             return null;
-        }, () -> cfText.isDone() ? true : false);
+        }, () -> cfText.isDone());
         webSocket.abort();
         assertFails(IOE, cfText);
         assertFails(IOE, cfPing);

test/jdk/java/net/httpclient/websocket/PendingTextPongClose.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -80,8 +80,9 @@ public class PendingTextPongClose extends PendingOperations {
             assertFails(ISE, webSocket.sendPong(ByteBuffer.allocate(125)));
             cfClose = webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "ok");
             assertHangs(cfClose);
+            assertNotDone(cfText);
             return null;
-        }, () -> cfText.isDone() ? true : false);
+        }, () -> cfText.isDone());
         webSocket.abort();
         assertFails(IOE, cfText);
         assertFails(IOE, cfPong);

test/jdk/java/net/httpclient/websocket/Support.java

@@ -33,6 +33,7 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
 import static org.testng.Assert.assertThrows;
+import static org.testng.Assert.assertFalse;
 
 public class Support {
 
@@ -43,6 +44,10 @@ public class Support {
         Support.assertCompletesExceptionally(clazz, stage);
     }
 
+    public static void assertNotDone(CompletableFuture<?> future) {
+        assertFalse(future.isDone());
+    }
+
     public static void assertCompletesExceptionally(Class<? extends Throwable> clazz,
                                                     CompletionStage<?> stage) {
         CompletableFuture<?> cf =