Skip to content
This repository has been archived by the owner. It is now read-only.
Permalink
Browse files
8249786: java/net/httpclient/websocket/PendingPingTextClose.java fail…
…s 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
  • Loading branch information
dfuch committed Aug 7, 2020
1 parent 21b9941 commit 1b6f67134bf2be04f9442d39fb66e49c7c9b76cf
Show file tree
Hide file tree
Showing 11 changed files with 98 additions and 51 deletions.
@@ -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 void closeOutput() throws IOException {
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 void closeInput() throws IOException {
channel.shutdownInput();
} finally {
if (outputClosed) {
ChannelState s = writeState.get();
assert s == CLOSED : s;
channel.close();
}
}
@@ -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 void pendingBinaryPingClose(boolean last) throws Exception {
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);
@@ -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 void pendingBinaryPongClose(boolean last) throws Exception {
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);
@@ -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 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 @@ static void assertFails(Class<? extends Throwable> clazz,
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 Object[][] booleans() {
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 @@ static void repeatable(Callable<Void> callable,
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;
@@ -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 void pendingPingBinaryClose(boolean last) throws Exception {
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);
@@ -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 @@

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( () -> {
server = Support.notReadingServer();
server.open();
webSocket = newBuilder().proxy(NO_PROXY).build().newWebSocketBuilder()
.buildAsync(server.getURI(), new WebSocket.Listener() { })
.join();
ByteBuffer data = ByteBuffer.allocate(125);
for (int i = 0; ; i++) { // fill up the send buffer
long start = System.currentTimeMillis();
System.out.printf("begin cycle #%s at %s%n", i, start);
cfPing = webSocket.sendPing(data);
try {
cfPing.get(MAX_WAIT_SEC, TimeUnit.SECONDS);
data.clear();
} catch (TimeoutException e) {
break;
} finally {
long stop = System.currentTimeMillis();
System.out.printf("end cycle #%s at %s (%s ms)%n", i, stop, stop - start);
try {
repeatable(() -> {
server = Support.notReadingServer();
server.open();
webSocket = newBuilder().proxy(NO_PROXY).build().newWebSocketBuilder()
.buildAsync(server.getURI(), new WebSocket.Listener() {
})
.join();
ByteBuffer data = ByteBuffer.allocate(125);
boolean done = false;
for (int i = 0; ; i++) { // fill up the send buffer
long start = System.currentTimeMillis();
if (debug) System.out.printf("begin cycle #%s at %s%n", i, start);
cfPing = webSocket.sendPing(data);
try {
cfPing.get(MAX_WAIT_SEC, TimeUnit.SECONDS);
data.clear();
} catch (TimeoutException e) {
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.sendPong(ByteBuffer.allocate(125)));
cfText = webSocket.sendText("hello", last);
assertHangs(cfText);
cfClose = webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "ok");
assertHangs(cfClose);
return null;
}, () -> cfPing.isDone() ? true : false);
webSocket.abort();
assertFails(IOE, cfPing);
assertFails(IOE, cfText);
assertFails(IOE, cfClose);
assertFails(ISE, webSocket.sendPing(ByteBuffer.allocate(125)));
assertFails(ISE, webSocket.sendPong(ByteBuffer.allocate(125)));
System.out.println("asserting that sendText hangs");
cfText = webSocket.sendText("hello", last);
assertHangs(cfText);
System.out.println("asserting that sendClose hangs");
cfClose = webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "ok");
assertHangs(cfClose);
System.out.println("asserting that cfPing is not completed");
assertNotDone(cfPing);
System.out.println("finishing");
return null;
}, () -> 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;
}
}
}
@@ -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 void pendingPongBinaryClose(boolean last) throws Exception {
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);
@@ -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 void pendingPongTextClose(boolean last) throws Exception {
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);
@@ -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 void pendingTextPingClose(boolean last) throws Exception {
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);
@@ -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 void pendingTextPongClose(boolean last) throws Exception {
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);
@@ -33,6 +33,7 @@
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 static void assertFails(Class<? extends Throwable> clazz,
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 =

0 comments on commit 1b6f671

Please sign in to comment.