RFR: 8347373: HTTP/2 flow control checks may count unprocessed data twice

[Daniel Fuchs]

On Fri, 10 Jan 2025 14:47:02 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> The HTTP/2 flow control logic has a potential race condition where some of the unprocessed data may be counted twice for the connection window.
>> A protocol exception may be raised incorrectly if there are several concurrent streams producing data and the connection window is close from being exhausted.
>
> src/java.net.http/share/classes/jdk/internal/net/http/WindowUpdateSender.java line 135:
> 
>> 133:     private boolean checkWindowSizeExceeded(long received, long len) {
>> 134:         // because windowSize is bound by Integer.MAX_VALUE
>> 135:         // we will never reach the point where received.get() + len
> 
> Nit - the method param `received` shadows the class level field and the comment that says "received.get()" would need to be reworded too. It might be better to rename the method param to something else.

I guess I could rename it to `processed`.

> src/java.net.http/share/classes/jdk/internal/net/http/WindowUpdateSender.java line 206:
> 
>> 204:                 int tosend = (int)Math.min(received.get(), Integer.MAX_VALUE);
>> 205:                 if (tosend > limit) {
>> 206:                     received.addAndGet(-tosend);
> 
> I'm guessing the change on this line is to merely be consistent with some others parts of the code in this class? Semantically, both these methods would be the same since we don't use the return value.

yes - I had added some println when debugging for which it was more convenient to call addAndGet than getAndAdd. As you say it's the same so I left it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23018#discussion_r1910497286
PR Review Comment: https://git.openjdk.org/jdk/pull/23018#discussion_r1910497406