RFR: 8302635: Race condition in HttpBodySubscriberWrapper when cancelling request [v2]

[Jaikiran Pai]

On Fri, 17 Feb 2023 14:05:06 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> src/java.net.http/share/classes/jdk/internal/net/http/common/HttpBodySubscriberWrapper.java line 172:
>> 
>>> 170:             // and closing the connection.
>>> 171:             userSubscriber.onError(t);
>>> 172:         }
>> 
>> I haven't fully wrapped my head around the possible flow of the user subscription, but is there a possiblity where this call to `onError(...)` here results in an reentrant call to this `propagateError()`? For that matter, not just reentrant but perhaps from a different thread concurrently? The reason I ask is, should we call this onError just once? The `java.util.concurrent.Flow.Subscriber.onError(Throwable)` method says this:
>>> Method invoked upon an unrecoverable error encountered by a
>>> Publisher or Subscription, after which no other Subscriber
>>> methods are invoked by the Subscription. 
>> 
>> So I'm wondering if we should maintain some state to only invoke it once?
>
> This is the purpose of this class: complete() and propagateError() should ensure that onError() is only called once in the wrapped subscriber. The markCompleted() call should ensure that, even if there is a reentrant call.

You are right - the error propagation through `propagateError()` always happens through the `complete()` method of this wrapper class and that method has the necessary state management to call this only once.

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

PR: https://git.openjdk.org/jdk/pull/12587