On Thu, 17 Nov 2022 09:56:40 GMT, Daniel Fuchs <dfu...@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
