On Mon, 25 Sep 2023 15:20:58 GMT, Daniel Fuchs <[email protected]> wrote:
>> Hi,
>>
>> Please find below a fix for 8316580: HttpClient with StructuredTaskScope
>> does not close when a task fails.
>>
>> The problem here is a subtle reference release issue: Interrupting the
>> VirtualThread means that the CompletableFuture returned by sendAsync is
>> eligible for GC after having been cancelled, which in turn means that some
>> of the intermediate operations that would have been completed before that CF
>> was completed get eligible for being GC’ed too. One of these intermediate
>> operations is the action that decrements the ref counting. Since the
>> refcount isn’t decremented properly, the client won’t exit.
>>
>> Holding onto the CompletableFuture returned to the caller by
>> HttpClient::sendAsync until that CompletableFuture gets completed from
>> upstream fixes the issue.
>
> Daniel Fuchs has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains six additional
> commits since the last revision:
>
> - Rename test, use TestTaskScope
> - Make the test independent of any preview API, and use regular threads
> - Improve the fix to make sure dependent actions are all executed
> - Merge branch 'master' into HttpGetWithCancelledStructuredScope-8316580
> - Merge branch 'master' into HttpGetWithCancelledStructuredScope-8316580
> - 8316580
test/jdk/java/net/httpclient/HttpGetInCancelledFuture.java line 124:
> 122: final Callable<T> task;
> 123: final CompletableFuture<T> cf = new CompletableFuture<>();
> 124: Task(Callable<T> task) {
Suggestion:
Task(Callable<T> task) {
test/jdk/java/net/httpclient/HttpGetInCancelledFuture.java line 184:
> 182: }
> 183:
> 184: protected <T> void completed(Task<T> task, T result, Throwable
> throwable) {
Suggestion:
protected <T> void completed(Task<T> task, T result, Throwable
throwable) {
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15883#discussion_r1336706948
PR Review Comment: https://git.openjdk.org/jdk/pull/15883#discussion_r1336707358
