From 94178f9361e911e236a41f1d491641b3eb5d60a1 Mon Sep 17 00:00:00 2001 From: Suren Gabrielyan Date: Fri, 12 Apr 2024 13:14:22 +0400 Subject: [PATCH] fix(websocket): Fix locking issues of `esp_websocket_client_send_with_exact_opcode` API --- .../esp_websocket_client.c | 60 +++++++++---------- .../target/sdkconfig.ci.dynamic_buffer | 14 +++++ 2 files changed, 42 insertions(+), 32 deletions(-) create mode 100644 components/esp_websocket_client/examples/target/sdkconfig.ci.dynamic_buffer diff --git a/components/esp_websocket_client/esp_websocket_client.c b/components/esp_websocket_client/esp_websocket_client.c index 9232cef3875..e437e5fb67b 100644 --- a/components/esp_websocket_client/esp_websocket_client.c +++ b/components/esp_websocket_client/esp_websocket_client.c @@ -552,9 +552,29 @@ static int esp_websocket_client_send_with_exact_opcode(esp_websocket_client_hand int wlen = 0, widx = 0; bool contained_fin = opcode & WS_TRANSPORT_OPCODES_FIN; + if (client == NULL || len < 0 || (data == NULL && len > 0)) { + ESP_LOGE(TAG, "Invalid arguments"); + return -1; + } + + if (!esp_websocket_client_is_connected(client)) { + ESP_LOGE(TAG, "Websocket client is not connected"); + return -1; + } + + if (client->transport == NULL) { + ESP_LOGE(TAG, "Invalid transport"); + return -1; + } + + if (xSemaphoreTakeRecursive(client->lock, timeout) != pdPASS) { + ESP_LOGE(TAG, "Could not lock ws-client within %" PRIu32 " timeout", timeout); + return -1; + } + if (esp_websocket_new_buf(client, true) != ESP_OK) { ESP_LOGE(TAG, "Failed to setup tx buffer"); - return -1; + goto unlock_and_return; } while (widx < len || opcode) { // allow for sending "current_opcode" only message with len==0 @@ -580,14 +600,18 @@ static int esp_websocket_client_send_with_exact_opcode(esp_websocket_client_hand esp_websocket_client_error(client, "esp_transport_write() returned %d, errno=%d", ret, errno); } esp_websocket_client_abort_connection(client, WEBSOCKET_ERROR_TYPE_TCP_TRANSPORT); - return ret; + goto unlock_and_return; } opcode = 0; widx += wlen; need_write = len - widx; } esp_websocket_free_buf(client, true); - return widx; + ret = widx; + +unlock_and_return: + xSemaphoreGiveRecursive(client->lock); + return ret; } esp_websocket_client_handle_t esp_websocket_client_init(const esp_websocket_client_config_t *config) @@ -1208,35 +1232,7 @@ int esp_websocket_client_send_fin(esp_websocket_client_handle_t client, TickType int esp_websocket_client_send_with_opcode(esp_websocket_client_handle_t client, ws_transport_opcodes_t opcode, const uint8_t *data, int len, TickType_t timeout) { - int ret = -1; - if (client == NULL || len < 0 || (data == NULL && len > 0)) { - ESP_LOGE(TAG, "Invalid arguments"); - return -1; - } - - if (xSemaphoreTakeRecursive(client->lock, timeout) != pdPASS) { - ESP_LOGE(TAG, "Could not lock ws-client within %" PRIu32 " timeout", timeout); - return -1; - } - - if (!esp_websocket_client_is_connected(client)) { - ESP_LOGE(TAG, "Websocket client is not connected"); - goto unlock_and_return; - } - - if (client->transport == NULL) { - ESP_LOGE(TAG, "Invalid transport"); - goto unlock_and_return; - } - - ret = esp_websocket_client_send_with_exact_opcode(client, opcode | WS_TRANSPORT_OPCODES_FIN, data, len, timeout); - if (ret < 0) { - ESP_LOGE(TAG, "Failed to send the buffer"); - goto unlock_and_return; - } -unlock_and_return: - xSemaphoreGiveRecursive(client->lock); - return ret; + return esp_websocket_client_send_with_exact_opcode(client, opcode | WS_TRANSPORT_OPCODES_FIN, data, len, timeout); } bool esp_websocket_client_is_connected(esp_websocket_client_handle_t client) diff --git a/components/esp_websocket_client/examples/target/sdkconfig.ci.dynamic_buffer b/components/esp_websocket_client/examples/target/sdkconfig.ci.dynamic_buffer new file mode 100644 index 00000000000..f74b16dac92 --- /dev/null +++ b/components/esp_websocket_client/examples/target/sdkconfig.ci.dynamic_buffer @@ -0,0 +1,14 @@ +CONFIG_IDF_TARGET="esp32" +CONFIG_IDF_TARGET_LINUX=n +CONFIG_WEBSOCKET_URI_FROM_STDIN=n +CONFIG_WEBSOCKET_URI_FROM_STRING=y +CONFIG_EXAMPLE_CONNECT_ETHERNET=y +CONFIG_EXAMPLE_CONNECT_WIFI=n +CONFIG_EXAMPLE_USE_INTERNAL_ETHERNET=y +CONFIG_EXAMPLE_ETH_PHY_IP101=y +CONFIG_EXAMPLE_ETH_MDC_GPIO=23 +CONFIG_EXAMPLE_ETH_MDIO_GPIO=18 +CONFIG_EXAMPLE_ETH_PHY_RST_GPIO=5 +CONFIG_EXAMPLE_ETH_PHY_ADDR=1 +CONFIG_EXAMPLE_CONNECT_IPV6=y +CONFIG_ESP_WS_CLIENT_ENABLE_DYNAMIC_BUFFER=y