RFR: 8268294: Reusing HttpClient in a WebSocket.Listener hangs.

[Daniel Fuchs]

On Wed, 16 Jun 2021 09:37:22 GMT, Michael McMahon <michaelm at openjdk.org> wrote:

>> test/jdk/java/net/httpclient/websocket/java.net.http/jdk/internal/net/http/websocket/WebSocketAndHttpClient.java line 38:
>> 
>>> 36:         HttpTest httpTest = new HttpTest(httpClient, args[1]);
>>> 37: 
>>> 38:         AtomicReference<String> result = new AtomicReference<>("failed");
>> 
>> Would it be possible to use a CountDownLatch - or a CompletableFuture<String> instead of an atomic reference in order to replace the Thread.sleep(3_000) at line 44 with latch.await() or cf.join()?
>> The test would then fail in jtreg timeout without the fix, but would not have to wait for an arbitrary magic 3s delay if the fix is present (which might not be enough delay on slow machines with debug builds etc...)
>
> Good idea on the CF or latch. As regards the blind cast, I suppose I could test for it and if the type is different, leave the executor reference as null, which reverts current behavior, but I can't imagine a scenario where that would actually happen. If it did, would we want to revert the behavior, or fail visibly with CCE?

I agree - I can't see how it could be something else than HttpClientFacade with the current implementation. Maybe just let's leave that up for later cleanup. What I had in mind was adding a static helper method somewhere on a public class in `jdk.internal.net.http` that would have access to the package private APIs - something like:


public static Executor getClientExecutor(HttpClient client) {
   ... and here we could do some switch depending on the concrete subclass of HttpClient
       we're dealing with ...
}


But I don't see an obvious home for such a method - so maybe we should leave this for a later cleanup.

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

PR: https://git.openjdk.java.net/jdk/pull/4506