espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.34k stars 7.21k forks source link

Near simultaneous calls to `httpd_ws_send_frame_async` causes frames to become mangled (IDFGH-13609) #14495

Open kevinhikaruevans opened 1 week ago

kevinhikaruevans commented 1 week ago

Answers checklist.

IDF version.

5.3.0

Espressif SoC revision.

ESP32-S3

Operating System used.

Linux

How did you build your project?

Other (please specify in More Information)

If you are using Windows, please specify command line type.

None

Development Kit.

NA

Power Supply used.

USB

What is the expected behavior?

Calling httpd_ws_send_frame_async in rapid succession should correctly queue the packets

What is the actual behavior?

The frames become mangled and the client(s) force a disconnect. Usually the client shows either a header error, ("Invalid frame header"), fragmentation error (final is incorrectly set to false), or a UTF8 decoding error ("Could not decode a text frame as UTF-8"). This behavior is also observable in Wireshark as errored frames.

Steps to reproduce.

  1. Call httpd_ws_send_frame_async to the same fd rapidly
  2. Observe that the client (web browser) disconnects due to an error

To resolve the problem, I can wrap the httpd_ws_send_frame_async calls in a freertos mutex. However, I think it would make more sense to be handling the mutex in the underlying functions.

Debug Logs.

No response

More Information.

I don't have a minimal example right now but I'm noticing this error when broadcasting messages to all ws clients (calling httpd_get_client_list and filtering). I'm only sending messages like up to 1-5 messages/sec, but it's enough to cause problems. I'm not sure if it would be better to queue frames or to use a simple mutex. I also wonder if combining the sends (via sess->send_fn()) would prevent the frames from being broken up?

kevinhikaruevans commented 1 week ago

My workaround looks something like:

    if (s_write_mutex != NULL && xSemaphoreTake(s_write_mutex, pdMS_TO_TICKS(1000)) == pdTRUE) {
        for (size_t i = 0; i < ws_count; i++) {
            if (fds[i] == req_fd) {
                // ignore the initiator
                continue;
            }
            err = httpd_ws_send_frame_async(*s_current_httpd_handle, fds[i], &frame);
            // [code omitted]
        }
        xSemaphoreGive(s_write_mutex);
    } else {
       // ...
    }

For reference, here's how the frame is being created:

    httpd_ws_frame_t frame = {
        .type = HTTPD_WS_TYPE_TEXT,
        .fragmented = false,
        .final = true,
        .len = strlen(str),
        .payload = (uint8_t*)str,
    };