On Wed, 16 Jun 2021 09:37:22 GMT, Michael McMahon <[email protected]> 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
