RFR: 8316580: HttpClient with StructuredTaskScope does not close when a task fails [v3]

[Daniel Fuchs]

On Mon, 25 Sep 2023 15:20:58 GMT, Daniel Fuchs <dfuchs at openjdk.org> 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

@djelinski has found out that the issue had nothing to do with GC, but that cancelling the last CompletableFuture in the chain actually caused the action leading to it (which unfortunately contained the refcount) to be popped off the stack.
So the fix is to always insert a dummy action before returning the completable future to the caller.
I have revised my fix to do just that - and refactored the code a bit to avoid inserting this action if not needed (e.g. when the future is already completed).

Since the issue has nothing to do with `StructuredTaskScope` or `VirtualThread` I have reworked the test to use a plain executor and to emulate a scope where an exception occurring in any of the concurrent tasks causes all others to be cancelled. The test still fails without the fix and passes with it.

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

PR Comment: https://git.openjdk.org/jdk/pull/15883#issuecomment-1733962253