On Mon, 9 Jun 2025 13:56:13 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> The java/net/httpclient/DigestEchoClient.java fails intemittently with >> IOException: HTTP/1.1 header parser received no bytes. >> >> Analysis shows that this is caused by the CleanupTrigger which receives data >> after the reused connection has been taken out of the HTTP/1.1 clear pool >> (Caused by: java.io.IOException: Data received while in pool). This should >> not happen. >> >> The fix for [JDK-8338569](https://bugs.openjdk.org/browse/JDK-8338569) has >> improved the situation but apparently didn't fix the issue completely. >> The issue is that the write publisher manages to come active before the read >> subscriber has been switched, which opens a window in which the old read >> subscriber receives the data that should have been passed to the new one. >> >> To fix that, this change proposes to pause reading at the selector level >> before connecting the flows with the new publisher /subscriber pair. This >> should ensure that data doesn't reach the "old" subscriber, if the new write >> publisher manage to send data before the new read subscriber is subscribed. > > 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 LGTM. 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. src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java line 956: > 954: ReadSubscription current = subscription; > 955: ReadSubscription pending = > pendingSubscriptions.getAndSet(null); > 956: if (pending != null) { Suggestion: if (pending != null) { ------------- Marked as reviewed by djelinski (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/25416#pullrequestreview-2912234681 PR Review Comment: https://git.openjdk.org/jdk/pull/25416#discussion_r2137077228 PR Review Comment: https://git.openjdk.org/jdk/pull/25416#discussion_r2137083771
