Bug 240311 - REGRESSION (STP): WebSocket.bufferedAmount does not get updated in some cases
Summary: REGRESSION (STP): WebSocket.bufferedAmount does not get updated in some cases
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Safari Technology Preview
Hardware: All Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-05-11 10:12 PDT by Roberto D'Auria
Modified: 2022-05-18 11:40 PDT (History)
6 users (show)

See Also:


Attachments
Patch (4.14 KB, patch)
2022-05-13 16:34 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (4.31 KB, patch)
2022-05-16 09:59 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (5.24 KB, patch)
2022-05-16 11:21 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (17.47 KB, patch)
2022-05-17 12:55 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roberto D'Auria 2022-05-11 10:12:18 PDT
This is probably related to the fix implemented for https://bugs.webkit.org/show_bug.cgi?id=235707.

Context:

The M-Lab team maintains a WebSocket-based network measurement client, https://speed.measurementlab.net/ (and elsewhere) which measures the maximum TCP throughput of the user’s Internet connection. The upload throughput measurement sends repeated messages over a WebSocket connection for 10 seconds. It uses bufferedAmount to monitor the backlog of untransmitted data. The goal is to always have some data ready and waiting to be transmitted but not so much data that the buffer overflows or takes too long to drain at the end of the test.


Overview:

We are testing the patch above, which makes the update of bufferedAmount synchronous, on the latest (version 144) Safari Technology Preview, and it looks like it might have introduced a significant regression. On some machines (more details below) bufferedAmount stops being updated after just a few writes, so our client stops sending data. Is it possible that bufferedAmount is not being decremented correctly in all the possible places in the code, or the data isn't actually sent over the network anymore for some reason?


Additional details:

The testing was done with the the code at https://github.com/m-lab/ndt7-js/blob/main/src/ndt7-upload-worker.js#L78 with added debugging logs around sock.send():

console.log("desired buffer: " + desiredBuffer + " - bufferedAmount: " + sock.bufferedAmount);
if (sock.bufferedAmount < desiredBuffer) {
         console.log("Sending " + data.length + " bytes");
         sock.send(data);
         console.log("Sent");
         total += data.length;
}

The very first calls to send() work normally. After flushing the buffer at least once (I can see bufferedAmount going down to zero), bufferedAmount reaches the desiredBuffer and then it’s never decremented again for the whole duration of the measurement -- 10 seconds. From the logs attached you can also see that NumBytes (total number of bytes sent) stays at 64KB and does not increase anymore.

This code works as intended on Safari stable (except for the sporadic crashes that caused us to report the original bug linked above) and every other major browser.

This new issue affects the speedtest at https://speed.measurementlab.net and Google's own speedtest client in search pages (OneBox), both based on websockets and the ndt7 protocol. Both of them report extremely low upload rates due to no new data being sent after the initial few milliseconds, matching what we observed in our testing. We believe that including that change as-is in the next Safari release will cause problems to all the integrations of the ndt7 speedtest protocol and everyone else relying on bufferedAmount to measure the backlog.

We have noticed differences in behavior depending on the hardware this was tested on:

2019 iMac 8-Core Intel i9, 64GB ram, MacOS Monterey: Works as intended
2019 MacBook Pro 8-core Intel i9 16GB ram, MacOS Monterey: Works as intended
2018 MacBook Pro 4-core Intel i7 16GB ram, MacOS Big Sur: Never works, stops sending after a few milliseconds as described above
2020 M1 MacBook Air, 16GB ram, MacOS Monterey: Works as intended when the tab running the measurement is in the foreground, stops sending when the tab is in the background. This is the most puzzling finding, but it could be replicated very consistently

Each test was run on a different network connection. Link speed does not seem to make a difference. Running the sender loop on the main thread vs a dedicated Web Worker does not seem to make any difference either.


Console logs, with comments added, collected on the 2018 MacBook Pro running Safari Technology Preview 144:

[Log] Upload started 
[Log] desired buffer: 57344 - bufferedAmount: 0 

First calls to send():

[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 8192 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 16384 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 24576 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 32768 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 40960 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 49152 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 57344 

bufferedAmount is decremented for the first time here:

[Log] desired buffer: 57344 - bufferedAmount: 24600 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 0 

After this point, bufferedAmount is not decremented anymore, as if no data is being sent over the network:

[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 8192 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 16384 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 24576 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 32768 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 40960 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 49152 
[Log] Sending 8192 bytes 
[Log] Sent 
[Log] desired buffer: 57344 - bufferedAmount: 57344 

From here on the client stops sending, since bufferedAmount == desiredBuffer. As a result, no new send() happens until the end of the 10-second measurement interval. What follows are just periodic measurement results which continue to decrease since no new data is sent:

[Log] Server-side measurement:  \'96 \{ConnectionInfo: \{Client: "<redacted>:52361", Server: "149.20.5.88:443", UUID: "<redacted>"\}, BBRInfo: Object, TCPInfo: Object\} 
\{ConnectionInfo: \{Client: "<redacted>:52361", Server: "149.20.5.88:443", UUID: "<redacted>"\}, BBRInfo: Object, TCPInfo: Object\}Object\
[Log] desired buffer: 57344 - bufferedAmount: 57344 
[Log] Client-side measurement:  \'96 \{ElapsedTime: 0.253, NumBytes: 65536, MeanClientMbps: 2.072284584980237\} 
[Log] desired buffer: 57344 - bufferedAmount: 57344 
[Log] Client-side measurement:  \'96 \{ElapsedTime: 0.507, NumBytes: 65536, MeanClientMbps: 1.0340986193293884\} 
[Log] desired buffer: 57344 - bufferedAmount: 57344 
[Log] Server-side measurement:  \'96 \{ConnectionInfo: \{Client: "<redacted>:52361", Server: "149.20.5.88:443", UUID: "<redacted>"\}, BBRInfo: Object, TCPInfo: Object\} 
\{ConnectionInfo: \{Client: "<redacted>:52361", Server: "149.20.5.88:443", UUID: "<redacted>"\}, BBRInfo: Object, TCPInfo: Object\}Object\
[Log] desired buffer: 57344 - bufferedAmount: 57344 
[Log] Client-side measurement:  \'96 \{ElapsedTime: 0.759, NumBytes: 65536, MeanClientMbps: 0.6907615283267456\} 
[Log] desired buffer: 57344 - bufferedAmount: 57344 
[Log] Server-side measurement:  \'96 \{ConnectionInfo: \{Client: "<redacted>:52361", Server: "149.20.5.88:443", UUID: "<redacted>"\}, BBRInfo: Object, TCPInfo: Object\} 

(... snip ...)
Comment 1 Radar WebKit Bug Importer 2022-05-11 10:16:12 PDT
<rdar://problem/93111593>
Comment 2 Alex Christensen 2022-05-13 16:34:23 PDT
Created attachment 459327 [details]
Patch
Comment 3 Alex Christensen 2022-05-13 16:35:41 PDT
Thanks for the report, Roberto.  Unfortunately, this means I need to restore the status quo on Big Sur, but fortunately newer OSes have been fixed.
Comment 4 Roberto D'Auria 2022-05-16 05:00:39 PDT
Thank you for looking into this!

Have you had a chance to verify the background vs foreground behavior on MacOS Monterey, too? It behaves the same as on Big Sur, but only when the tab running the send loop does not have focus.
Comment 5 Alex Christensen 2022-05-16 09:59:09 PDT
Created attachment 459433 [details]
Patch
Comment 6 Alex Christensen 2022-05-16 10:56:54 PDT
Roberto, I can't reproduce the background vs foreground issue on Monterey on my m1 MacBook Air with 16GB of ram.  Maybe I'm misunderstanding something, but I'm going to https://speed.measurementlab.net/#/ then starting the speed test and switching to another tab or minimizing Safari, and I see no difference.  It's definitely a different issue than the one this bug is fixing, though.
Comment 7 Alex Christensen 2022-05-16 11:21:00 PDT
Created attachment 459439 [details]
Patch
Comment 8 Roberto D'Auria 2022-05-16 12:41:50 PDT
The procedure is the correct one, but we're having some trouble replicating it ourselves right now. I'll open a separate issue if it happens again and I can figure out how to replicate it more consistently.

It's also possible that it can happen on Safari stable too, but rarely enough that we never noticed. Thanks!
Comment 9 EWS 2022-05-16 13:50:37 PDT
Found 16 new test failures: imported/w3c/web-platform-tests/websockets/Send-65K-data.any.html, imported/w3c/web-platform-tests/websockets/Send-binary-65K-arraybuffer.any.html, imported/w3c/web-platform-tests/websockets/Send-binary-65K-arraybuffer.any.worker.html, imported/w3c/web-platform-tests/websockets/Send-binary-arraybuffer.any.html, imported/w3c/web-platform-tests/websockets/Send-binary-arraybuffer.any.worker.html, imported/w3c/web-platform-tests/websockets/Send-data.any.html, imported/w3c/web-platform-tests/websockets/Send-data.any.worker.html, imported/w3c/web-platform-tests/websockets/Send-paired-surrogates.any.html, imported/w3c/web-platform-tests/websockets/Send-paired-surrogates.any.worker.html, imported/w3c/web-platform-tests/websockets/Send-unicode-data.any.worker.html ...
Comment 10 Alex Christensen 2022-05-17 12:55:49 PDT
Created attachment 459512 [details]
Patch
Comment 11 EWS 2022-05-18 11:40:18 PDT
Committed r294413 (250705@main): <https://commits.webkit.org/250705@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 459512 [details].