[tobfah]

This PR fixes a potential unsigned wrap-around for the congestion window which has the unintended consequence of setting it to the maximum window value as opposed to keeping it low.

This also avoids double adjustment of the window by making the two branches (latency spike and congestion avoidance) mutually exclusive.

[CendioOssman]

When does this occur? Can it be easily tested?

[CendioOssman]

It looks like your commit email doesn't match what you have registered at GitHub? The commit isn't attributed to you.

[tobfah]

It looks like your commit email doesn't match what you have registered at GitHub? The commit isn't attributed to you.

Looks like the domain used in my local configuration (.se) didn't match my github account (.com). Should be fixed now.

[tobfah]

When does this occur?

It happens when the server detects a latency spike (meeting a certain condition) and the congestion window happens to currently sit at a value such that the proportional adjustment factor baseRTT / minRTT would bring it below 4 KiB.

The ratio above is between the estimated base latency of the network (baseRTT) and the minimum measured latency (minRTT) since the last window adjustment (based on a short series of measurements).

As seen in Congestion::updateCongestion(), if diff = minRTT - baseRTT fulfills the condition diff > 100 && diff > baseRTT/2 we'll apply the adjustment factor baseRTT / minRTT and then this invariably falls through to an additional 4 KiB subtraction from congWindow further down since at that point inSlowStart = false and diff > 50 is implied from the earlier diff > 100.

If the first adjustment sent congWindow below 4 KiB then the subsequent 4 KiB subtraction wraps it around to somewhere close to its unsigned maximum. This is then clamped to the MAXIMUM_WINDOW value of 4096 KiB towards the end.

Can it be easily tested?

I've managed to reproduce this a couple of times by having the server send quite small screen updates (some text fluttering in the corner of a terminal window) on an unthrottled network and then suddenly throttling it to 300ms.

With some added debug logging I could observe for instance the congestion window at 128 KiB on entering Congestion::updateCongestion() with baseRTT at 1 ms and minRTT at ~300 ms. The window was adjusted to roughly 400 bytes via the ~1/300 factor, and subsequently wrapped to a value not far from the unsigned maximum of congWindow via the 4 KiB subtraction, and finally clamped to the MAXIMUM_WINDOW value at the end.