fix(websocket): release client-lock during WEBSOCKET_EVENT_DATA (IDFGH-14545)

[bryghtlabs-richard]

Description

Prevent deadlocks when reserving locks in WEBSOCKET_EVENT_DATA handler, and lock is held by another thread sending a websocket message.

Fix high latency caused by writers serialized with WEBSOCKET_EVENT_DATA while calling esp_websocket_client_send(), even when TCP window has enough space for the entire message being queued to send.

Multiple writers are still serialized at fragment boundaries, but only with other writers and websocket error updates.

Related

Fixes #625.

Testing

I tested with a WebSocket echo server running on the same WiFi access point, using the target-example with the following modifications:

• AppMain thread tries to send 20 unfragmented frames, waiting 100ms between frames.
• WebSocket client is patched to record the time taken to send each frame.
• WEBSOCKET_EVENT_DATA handler calls vTaskDelay(configTICK_RATE_HZ); to simulate application-processing.

Without this patch, the AppMain thread experiences long delays, often either almost 1 second, and sometimes much longer(like 6 seconds) depending on which cores the AppMain and WebSocketClient thread run. The AppMain thread is unable to send at 10Hz, even though the TCP send buffer is never full.

With this patch, the AppMain thread experiences no delays writing websocket frames, and is able to send at 10Hz, as long as the TCP send buffer is not filled.

Examples logs:
test_before_fix.txt
test_before_fix2.txt
test_with_fix.txt

Checklist

Before submitting a Pull Request, please ensure the following:

• 🚨 This PR does not introduce breaking changes.
• All CI checks (GH Actions) pass.
• Documentation is updated as needed.
• Tests are updated or added as necessary.
• Code is well-commented, especially in complex areas.
• Git history is clean — commits are squashed to the minimum necessary.

[erwinschrodinger1]

There was similar issue when I tried the esp_websocket_client. This PR commit solves the issue of sending in the socket but I am not being able to get the message response from the server.

The message data case is never called.

Thank you @bryghtlabs-richard for solving the send issue.

[erwinschrodinger1]

Hello @bryghtlabs-richard. I tried using this after you did the another push. Then the same issue of unable to lock resembles which was solved after the first push.

Thank you.

[bryghtlabs-richard]

@erwinschrodinger1, if you try to send WS data with timeout=0 while the WS client is locked by the receive thread, xSemaphoreTakeRecursive(client->lock, timeout=0) will immediately fail. You might have better luck with a small, non-zero timeout. It is normal for the WS client receive thread to lock the client->lock for short periods while processing.

[erwinschrodinger1]

@bryghtlabs-richard how should I solve my error then? Use a small delay after sending message each time?

[bryghtlabs-richard]

@erwinschrodinger1 , I wouldn't add a separate delay, but I would try a small timeout, maybe around 100ms.

If a thread tries to send while the client is locked, xSemaphoreTakeRecursive() will delay up to the timeout, and if the client is unlocked sooner then the sending thread can continue sooner.

If a thread tries to send while the client is unlocked, the sender can lock/send/unlock without delay.

[erwinschrodinger1]

Sorry initially I mistook ticks with ms. Even after adding the timeout of around 10 second. The ws-client is not being able to lock. What can be the issue in it?

[bryghtlabs-richard]

I would guess the client is either not setup properly, or lock is taken but not released. I can't think of anything that would take 10 seconds. Its possible a thread has locked it and forgotten to unlock it, but I don't see how this change would do it.

The next thing I'd want to know - is client->lock locked? If so, by which line of code?

[erwinschrodinger1]

Thank you so much @bryghtlabs-richard.

the issue was done by my handler which locked the WEBSOCKET_DATA event resulting in lock of client. It is was just me and my idiotic acts. Sorry.

[bryghtlabs-richard]

No worries @erwinschrodinger1 , thanks for letting us know the cause was unrelated to this patchset.