[Webkit-unassigned] [Bug 235707] New: WebSocket.send() overflows the buffer but bufferedAmount is zero

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=235707

            Bug ID: 235707
           Summary: WebSocket.send() overflows the buffer but
                    bufferedAmount is zero
           Product: WebKit
           Version: Safari 15
          Hardware: All
                OS: All
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: JavaScriptCore
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: roberto at measurementlab.net

Overview:

Checking the bufferedAmount property is not sufficient to prevent send() from crashing with “Failed to send WebSocket frame” while attempting to maintain a steady send backlog. The code described below works as intended on many platforms but not the iPhone 11s and other fast MacOS devices running Safari.

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.

This technique works well on a large number of platforms (Chrome/Chromium, Firefox, Opera and Edge on both desktop and mobile where applicable). except it fails on the iPhone 11s and other fast Apple devices. We had previously implemented a workaround by limiting the desired backlog size to a value much smaller than the maximum buffer size. The problem seems to be that the bufferedAmount property is not updated in a timely manner following a send(), resulting in overrunning the buffer on faster devices.

Additional details:

The message size starts very small (8KB) and is doubled over time up to a maximum of 8MB within the initial 2-3 seconds of the measurement. We aim to keep 8 messages in the buffer at all times. To determine whether we can push more messages into the WebSocket buffer without exceeding its maximum size, we compare the bufferedAmount property of the WebSocket object with a desiredBuffer (8 * data.length, so up to 64 MB).

We received several reports of the upload function crashing on WebKit-based browsers (specifically, Safari) due to the code attempting to call WebSocket.send() when the buffer is full (or would exceed its maximum size after the send()).

While debugging this issue, we noticed that the bufferedAmount property does not seem to be updated immediately after a send() but it’s updated asynchronously and lags behind in a non-predictable way, not reflecting the actual state of the buffer at the time the property is checked. This causes in some cases (depending on the user’s hardware, connection speed, and latency) the algorithm to attempt to send more data than what the WS buffer can hold at the moment and the connection to be abruptly terminated during a measurement with a "Failed to send WebSocket frame" error on the Javascript console.

You can see some of the investigation that was made and further details at:

- https://github.com/m-lab/ndt7-js/issues/44#issuecomment-839912733
- https://www.measurementlab.net/blog/ndt7-updates/

As you can see from the comments to the GitHub issue above and as we confirmed in our testing, it is possible for bufferedAmount to be reported as zero at the time of the check but the subsequent call to sock.send(data) fails -- we suspect because sending data would overflow the buffer.

We attempted to work around this issue by basically slowing down the send loop and queuing fewer messages, as you can see in this PR:
https://github.com/m-lab/ndt7-js/pull/45/files

While this seemed to work reasonably well initially, we are still getting reports from our users about the upload function crashing intermittently.

This is the relevant part of the uploader function’s code (full code at https://github.com/m-lab/ndt7-js/blob/main/src/ndt7-upload-worker.js), which works as expected on all the other browsers listed above.


function uploader(data, start, end, previous, total) {
    // [... part of the function omitted ...]

    const maxMessageSize = 8388608; /* = (1<<23) = 8MB */

    // Message size is doubled after the first 16 messages, and subsequently
    // every 8, up to maxMessageSize.
    const nextSizeIncrement =
        (data.length >= maxMessageSize) ? Infinity : 16 * data.length;

    // Determine whether it's time to double the message size.
    if ((total - sock.bufferedAmount) >= nextSizeIncrement) {
      data = new Uint8Array(data.length * 2);
    }

    // We keep 7 messages in the send buffer, so there is always some more
    // data to send. The maximum buffer size is 7 * 8MB - 1 byte ~= 56M.
    // Note: this was previously 8 messages - changed to 7 in an attempt to work around this issue.
    const desiredBuffer = 7 * data.length;

    // XXX: on WebKit this check can succeed but the subsequent sock.send(data) will fail.
    if (sock.bufferedAmount < desiredBuffer) {
      sock.send(data);
      total += data.length;
    }

    // Loop the uploader function in a way that respects the JS event handler.
    setTimeout(() => uploader(data, start, end, previous, total), 0);
  }

We have been looking for similar issues with bufferedAmount reported in the past. The only issue we found was a regression in 2017 (https://bugs.webkit.org/show_bug.cgi?id=170463) where the bufferedAmount property didn't seem to be updated at all, which is marked as RESOLVED FIXED.

Actual results:
The "bufferedAmount" property reports a value of zero (or small enough for another send() to be called). After send(), the WebSocket connection crashes with a "Failed to send WebSocket frame" error.

Expected results:
As long as the backlog is smaller than (maximum WebSocket buffer size - data.length), the next send(data) should always succeed.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220127/7c23939f/attachment.htm>