http-client-branch: (WebSocket) remove close timer http-client-branch
authorprappo
Wed, 21 Mar 2018 10:25:26 +0000
branchhttp-client-branch
changeset 56327 efd50952acd9
parent 56326 63422db47911
child 56328 161f716753d7
http-client-branch: (WebSocket) remove close timer
src/java.net.http/share/classes/jdk/internal/net/http/websocket/MessageEncoder.java
src/java.net.http/share/classes/jdk/internal/net/http/websocket/WebSocketImpl.java
test/jdk/java/net/httpclient/websocket/SendTest.java
--- a/src/java.net.http/share/classes/jdk/internal/net/http/websocket/MessageEncoder.java	Tue Mar 20 13:10:09 2018 +0000
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/websocket/MessageEncoder.java	Wed Mar 21 10:25:26 2018 +0000
@@ -318,11 +318,11 @@
                 try {
                     r.throwException();
                 } catch (CharacterCodingException e) {
-                    throw new IllegalArgumentException("Malformed reason", e);
+                    throw new IOException("Malformed reason", e);
                 }
             } else if (r.isOverflow()) {
                 // Here the 125 bytes size is ensured by the check for overflow
-                throw new IllegalArgumentException("Long reason");
+                throw new IOException("Long reason");
             } else if (!r.isUnderflow()) {
                 throw new InternalError(); // assertion
             }
--- a/src/java.net.http/share/classes/jdk/internal/net/http/websocket/WebSocketImpl.java	Tue Mar 20 13:10:09 2018 +0000
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/websocket/WebSocketImpl.java	Wed Mar 21 10:25:26 2018 +0000
@@ -48,8 +48,6 @@
 import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionStage;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
@@ -97,7 +95,6 @@
 
     private final AtomicReference<ByteBuffer> lastAutomaticPong = new AtomicReference<>();
     private final MinimalFuture<WebSocket> DONE = MinimalFuture.completedFuture(this);
-    private final long closeTimeout;
     private volatile boolean inputClosed;
     private final AtomicBoolean outputClosed = new AtomicBoolean();
 
@@ -165,25 +162,6 @@
         this.listener = requireNonNull(listener);
         this.transport = transportFactory.createTransport(
                 new SignallingMessageConsumer());
-        closeTimeout = readCloseTimeout();
-    }
-
-    private static int readCloseTimeout() {
-        String property = "jdk.httpclient.websocket.closeTimeout";
-        int defaultValue = 30;
-        String value = Utils.getNetProperty(property);
-        int v;
-        if (value == null) {
-            v = defaultValue;
-        } else {
-            try {
-                v = Integer.parseUnsignedInt(value);
-            } catch (NumberFormatException ignored) {
-                v = defaultValue;
-            }
-        }
-        debug.log(Level.DEBUG, "%s=%s, using value %s", property, value, v);
-        return v;
     }
 
     // FIXME: add to action handling of errors -> signalError()
@@ -346,50 +324,20 @@
      */
     private CompletableFuture<WebSocket> sendClose0(int statusCode,
                                                     String reason) {
-        // TODO: timeout on onClose receiving
-        CompletableFuture<WebSocket> cf
-                = transport.sendClose(statusCode, reason, this, (r, e) -> { });
-        CompletableFuture<WebSocket> closeOrTimeout
-                = replaceNull(cf).orTimeout(closeTimeout, TimeUnit.SECONDS);
-        // The snippet below, whose purpose might not be immediately obvious,
-        // is a trick used to complete a dependant stage with an IOException.
-        // A checked IOException cannot be thrown from inside the BiConsumer
-        // supplied to the handle method. Instead a CompletionStage completed
-        // exceptionally with this IOException is returned.
-        return closeOrTimeout.handle(this::processCloseOutcome)
-                             .thenCompose(Function.identity());
+        return transport.sendClose(statusCode, reason, this,
+                                   (r, e) -> processCloseError(e));
     }
 
-    private CompletionStage<WebSocket> processCloseOutcome(WebSocket webSocket,
-                                                           Throwable e) {
+    private void processCloseError(Throwable e) {
         if (e == null) {
             debug.log(Level.DEBUG, "send close completed successfully");
         } else {
             debug.log(Level.DEBUG, "send close completed with error", e);
         }
-        if (e == null) {
-            try {
-                transport.closeOutput();
-            } catch (IOException ignored) { }
-            return completedFuture(webSocket);
-        }
-        Throwable cause = Utils.getCompletionCause(e);
-        if (cause instanceof IllegalArgumentException) {
-            return failedFuture(cause);
-        }
-        if (cause instanceof TimeoutException) {
-            outputClosed.set(true);
-            try {
-                transport.closeOutput();
-            } catch (IOException ignored) { }
-            inputClosed = true;
-            try {
-                transport.closeInput();
-            } catch (IOException ignored) { }
-            return failedFuture(new InterruptedIOException(
-                    "Could not send close within a reasonable timeout"));
-        }
-        return failedFuture(cause);
+        outputClosed.set(true);
+        try {
+            transport.closeOutput();
+        } catch (IOException ignored) { }
     }
 
     @Override
--- a/test/jdk/java/net/httpclient/websocket/SendTest.java	Tue Mar 20 13:10:09 2018 +0000
+++ b/test/jdk/java/net/httpclient/websocket/SendTest.java	Wed Mar 21 10:25:26 2018 +0000
@@ -189,40 +189,4 @@
         assertTrue(webSocket.isInputClosed());
         Support.assertFails(IOException.class, cf);
     }
-
-    @Test // FIXME: TO BE REMOVED as we agreed upon no timeout in sendClose
-    public void sendCloseTimeout() throws Exception {
-        server = Support.notReadingServer();
-        server.open();
-        webSocket = newHttpClient()
-                .newWebSocketBuilder()
-                .buildAsync(server.getURI(), new WebSocket.Listener() { })
-                .join();
-        String data = Support.stringWith2NBytes(32768);
-        CompletableFuture<WebSocket> cf = null;
-        for (int i = 0; ; i++) {  // fill up the send buffer
-            System.out.printf("begin cycle #%s at %s%n",
-                              i, System.currentTimeMillis());
-            try {
-                cf = webSocket.sendText(data, true);
-                cf.get(10, TimeUnit.SECONDS);
-            } catch (TimeoutException e) {
-                break;
-            } finally {
-                System.out.printf("end cycle #%s at %s%n",
-                                  i, System.currentTimeMillis());
-            }
-        }
-        long before = System.currentTimeMillis();
-        Support.assertFails(IOException.class,
-                            webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "ok"));
-        long after = System.currentTimeMillis();
-        // default timeout should be 30 seconds
-        long elapsed = after - before;
-        System.out.printf("Elapsed %s ms%n", elapsed);
-        assertTrue(elapsed >= 29_000, String.valueOf(elapsed));
-        assertTrue(webSocket.isOutputClosed());
-        assertTrue(webSocket.isInputClosed());
-        Support.assertFails(IOException.class, cf);
-    }
 }