RFR: 8357639: DigestEchoClient fails intermittently due to: java.io.IOException: Data received while in pool [v4]
Daniel Jeliński
djelinski at openjdk.org
Tue Jun 10 07:11:31 UTC 2025
On Mon, 9 Jun 2025 13:56:13 GMT, Daniel Fuchs <dfuchs at 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
More information about the net-dev
mailing list