Skip to content

Commit 46871bf

Browse files
committed
fix(websocket): Fix reconnection data loss and memory leaks
1. Reset frame parsing state (payload_len, payload_offset, last_opcode) on new connection 2. Free errormsg_buffer in esp_websocket_client_disconnect() 3. Destroy transport_list immediately in esp_websocket_client_stop 4. Added sdkconfig.ci.tx_lock config
1 parent d5a2836 commit 46871bf

File tree

2 files changed

+50
-3
lines changed

2 files changed

+50
-3
lines changed

components/esp_websocket_client/esp_websocket_client.c

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,9 @@ static esp_err_t esp_websocket_client_abort_connection(esp_websocket_client_hand
245245
{
246246
ESP_WS_CLIENT_STATE_CHECK(TAG, client, return ESP_FAIL);
247247

248+
// Note: This function must be called with client->lock already held
249+
// The caller is responsible for acquiring the lock before calling
250+
248251
// CRITICAL: Check if already closing/closed to prevent double-close
249252
if (client->state == WEBSOCKET_STATE_CLOSING || client->state == WEBSOCKET_STATE_UNKNOW) {
250253
ESP_LOGW(TAG, "Connection already closing/closed, skipping abort");
@@ -267,6 +270,16 @@ static esp_err_t esp_websocket_client_abort_connection(esp_websocket_client_hand
267270
client->error_handle.error_type = error_type;
268271
esp_websocket_client_dispatch_event(client, WEBSOCKET_EVENT_DISCONNECTED, NULL, 0);
269272

273+
if (client->errormsg_buffer) {
274+
ESP_LOGI(TAG, "Freeing error buffer (%d bytes) - Free heap: %" PRIu32 " bytes",
275+
client->errormsg_size, esp_get_free_heap_size());
276+
free(client->errormsg_buffer);
277+
client->errormsg_buffer = NULL;
278+
client->errormsg_size = 0;
279+
} else {
280+
ESP_LOGI(TAG, "Disconnect - Free heap: %" PRIu32 " bytes", esp_get_free_heap_size());
281+
}
282+
270283
return ESP_OK;
271284
}
272285

@@ -1081,11 +1094,15 @@ static esp_err_t esp_websocket_client_recv(esp_websocket_client_handle_t client)
10811094
xSemaphoreGiveRecursive(client->tx_lock);
10821095
return ESP_OK; // Caller expects client->lock to be held, which it is
10831096
}
1084-
#endif
1097+
10851098
esp_transport_ws_send_raw(client->transport, WS_TRANSPORT_OPCODES_PONG | WS_TRANSPORT_OPCODES_FIN, data, client->payload_len,
10861099
client->config->network_timeout_ms);
1087-
#ifdef CONFIG_ESP_WS_CLIENT_SEPARATE_TX_LOCK
10881100
xSemaphoreGiveRecursive(client->tx_lock);
1101+
#else
1102+
// When separate TX lock is not configured, we already hold client->lock
1103+
// which protects the transport, so we can send PONG directly
1104+
esp_transport_ws_send_raw(client->transport, WS_TRANSPORT_OPCODES_PONG | WS_TRANSPORT_OPCODES_FIN, data, client->payload_len,
1105+
client->config->network_timeout_ms);
10891106
#endif
10901107
} else if (client->last_opcode == WS_TRANSPORT_OPCODES_PONG) {
10911108
client->wait_for_pong_resp = false;
@@ -1183,6 +1200,11 @@ static void esp_websocket_client_task(void *pv)
11831200
client->state = WEBSOCKET_STATE_CONNECTED;
11841201
client->wait_for_pong_resp = false;
11851202
client->error_handle.error_type = WEBSOCKET_ERROR_TYPE_NONE;
1203+
client->payload_len = 0;
1204+
client->payload_offset = 0;
1205+
client->last_fin = false;
1206+
client->last_opcode = WS_TRANSPORT_OPCODES_NONE;
1207+
11861208
esp_websocket_client_dispatch_event(client, WEBSOCKET_EVENT_CONNECTED, NULL, 0);
11871209
break;
11881210
case WEBSOCKET_STATE_CONNECTED:
@@ -1273,6 +1295,7 @@ static void esp_websocket_client_task(void *pv)
12731295
xSemaphoreTakeRecursive(client->lock, lock_timeout);
12741296
if (esp_websocket_client_recv(client) == ESP_FAIL) {
12751297
ESP_LOGE(TAG, "Error receive data");
1298+
// Note: Already holding client->lock from line above
12761299
esp_websocket_client_abort_connection(client, WEBSOCKET_ERROR_TYPE_TCP_TRANSPORT);
12771300
}
12781301
xSemaphoreGiveRecursive(client->lock);
@@ -1357,7 +1380,16 @@ esp_err_t esp_websocket_client_stop(esp_websocket_client_handle_t client)
13571380
return ESP_FAIL;
13581381
}
13591382

1360-
return stop_wait_task(client);
1383+
esp_err_t ret = stop_wait_task(client);
1384+
1385+
if (client->transport_list) {
1386+
ESP_LOGI(TAG, "Destroying transport list to free resources immediately");
1387+
esp_transport_list_destroy(client->transport_list);
1388+
client->transport_list = NULL;
1389+
client->transport = NULL;
1390+
}
1391+
1392+
return ret;
13611393
}
13621394

13631395
static int esp_websocket_client_send_close(esp_websocket_client_handle_t client, int code, const char *additional_data, int total_len, TickType_t timeout)
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
CONFIG_IDF_TARGET="esp32"
2+
CONFIG_IDF_TARGET_LINUX=n
3+
CONFIG_WEBSOCKET_URI_FROM_STDIN=n
4+
CONFIG_WEBSOCKET_URI_FROM_STRING=y
5+
CONFIG_EXAMPLE_CONNECT_ETHERNET=y
6+
CONFIG_EXAMPLE_CONNECT_WIFI=n
7+
CONFIG_EXAMPLE_USE_INTERNAL_ETHERNET=y
8+
CONFIG_EXAMPLE_ETH_PHY_IP101=y
9+
CONFIG_EXAMPLE_ETH_MDC_GPIO=23
10+
CONFIG_EXAMPLE_ETH_MDIO_GPIO=18
11+
CONFIG_EXAMPLE_ETH_PHY_RST_GPIO=5
12+
CONFIG_EXAMPLE_ETH_PHY_ADDR=1
13+
CONFIG_EXAMPLE_CONNECT_IPV6=y
14+
CONFIG_ESP_WS_CLIENT_SEPARATE_TX_LOCK=y
15+
CONFIG_ESP_WS_CLIENT_TX_LOCK_TIMEOUT_MS=2000

0 commit comments

Comments
 (0)