On Tue, 10 Jun 2025 07:03:06 GMT, Daniel Jeliński <[email protected]> wrote:
>> 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 eight additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into DigestEchoClient-cleanup-8357639
>> - Logging
>> - A better fix
>> - Merge branch 'master' into DigestEchoClient-cleanup-8357639
>> - Access pendingSubscriptions only from InternalReadSubscriptionImpl - and
>> synchronize offer/handle to avoid concurrent removal from the queue
>> - Merge branch 'master' into DigestEchoClient-cleanup-8357639
>> - Avoid unnecessary volatile reads
>> - 8357639
>
> src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java line
> 931:
>
>> 929: synchronized void offer(TubeSubscriber sub) {
>> 930: ReadSubscription target = new ReadSubscription(this,
>> sub);
>> 931: ReadSubscription previous =
>> pendingSubscriptions.getAndSet(target);
>
> now that all access to pendingSubscription is synchronized, it doesn't need
> to be an AtomicReference, a regular field will work just as well.
Ah, good point. But it makes it possible to use getAndSet(). I'm inclined to
keep it just for that.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25416#discussion_r2137417591
