RFR: 8348108: Race condition in AggregatePublisher.AggregateSubscription [v4]

[Daniel Fuchs]

On Tue, 21 Jan 2025 18:33:50 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> There's a subtle race condition in AggregatePublisher.AggregateSubscription. The AggregatePublisher will subscribe to downstream publishers in turn, subscribing to the next publisher when the previous publisher onComplete() has been called.
>> 
>> The request() method passed to the upstream subscriber detects that all subscribers have completed if the current downstream publisher is null and the queue of publisher yet to subscribe to is empty.
>> 
>> The event loop is responsible for subscribing to the next publisher, and does so by polling the queue and assigning the returned value to this.publisher, and then subscribe to that publisher. However, when subscribing to the last publisher in the queue, there is a small time window where the queue can be observed to be empty, before this.publisher is assigned.
>> 
>> The fix adds synchronization to make sure a consistent state is observed when it matters. Typically - setting `this.publisher` or taking a snapshot of a publisher and its subscription, should be done within synchronized.
>
> 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 five additional commits since the last revision:
> 
>  - Merge branch 'master' into AggregatePublisher-8348108
>  - Fields could be final in test class
>  - Review feedback
>  - Merge branch 'master' into AggregatePublisher-8348108
>  - 8348108: Race condition in AggregatePublisher.AggregateSubscription

src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java line 575:

> 573:                 subscription.cancel();
> 574:             }
> 575:             // This nethod is called when cancel is true, so

Suggestion:

            // This method is called when cancel is true, so

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23204#discussion_r1925529344