RFR: 8297149: REDO JDK-8296889: Race condition when cancelling a request [v2]

[Jaikiran Pai]

On Thu, 17 Nov 2022 09:56:40 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> src/java.net.http/share/classes/jdk/internal/net/http/common/HttpBodySubscriberWrapper.java line 188:
>> 
>>> 186:         // race condition with propagateError: we need to wait until
>>> 187:         // subscription is finished before calling onError;
>>> 188:         subscriptionLock.lock();
>> 
>> More of a question than a review comment - I see that the only place in this class where we were using `synchronized` is while dealing with the `subscribed`. The PR replaces the `synchronized` blocks with a `ReentrantLock`. Does that have an advantage in context of this code?
>
> Well - I am not sure. The code within the block calls onSubscribe on the user subscriber. Theoretically, this should not block, but it might kick off a loop (via calling subscription::request) that may run in the current thread for a while. Using a lock ensures that the waiter will be well-behaved for virtual threads if that ever happens - and no carrier thread will be pinned because of this.

Thank you for that detail. I was suspecting the virtual thread usecase is what might have prompted this but wanted to be sure.

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

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