Skip to content

Commit 5577e03

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. Added sdkconfig.ci.tx_lock config
1 parent d5a2836 commit 5577e03

File tree

2 files changed

+42
-5
lines changed

2 files changed

+42
-5
lines changed

components/esp_websocket_client/esp_websocket_client.c

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -245,16 +245,16 @@ 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");
251254
return ESP_OK;
252255
}
253256

254257
esp_transport_close(client->transport);
255-
// Note: We do NOT set client->transport = NULL here because:
256-
// 1. Transport is reused on reconnect
257-
// 2. We use state instead to track if transport is valid
258258

259259
if (!client->config->auto_reconnect) {
260260
client->run = false;
@@ -267,6 +267,16 @@ static esp_err_t esp_websocket_client_abort_connection(esp_websocket_client_hand
267267
client->error_handle.error_type = error_type;
268268
esp_websocket_client_dispatch_event(client, WEBSOCKET_EVENT_DISCONNECTED, NULL, 0);
269269

270+
if (client->errormsg_buffer) {
271+
ESP_LOGI(TAG, "Freeing error buffer (%d bytes) - Free heap: %" PRIu32 " bytes",
272+
client->errormsg_size, esp_get_free_heap_size());
273+
free(client->errormsg_buffer);
274+
client->errormsg_buffer = NULL;
275+
client->errormsg_size = 0;
276+
} else {
277+
ESP_LOGI(TAG, "Disconnect - Free heap: %" PRIu32 " bytes", esp_get_free_heap_size());
278+
}
279+
270280
return ESP_OK;
271281
}
272282

@@ -464,6 +474,8 @@ static void destroy_and_free_resources(esp_websocket_client_handle_t client)
464474
esp_websocket_client_destroy_config(client);
465475
if (client->transport_list) {
466476
esp_transport_list_destroy(client->transport_list);
477+
client->transport_list = NULL;
478+
client->transport = NULL;
467479
}
468480
vSemaphoreDelete(client->lock);
469481
#ifdef CONFIG_ESP_WS_CLIENT_SEPARATE_TX_LOCK
@@ -1081,11 +1093,15 @@ static esp_err_t esp_websocket_client_recv(esp_websocket_client_handle_t client)
10811093
xSemaphoreGiveRecursive(client->tx_lock);
10821094
return ESP_OK; // Caller expects client->lock to be held, which it is
10831095
}
1084-
#endif
1096+
10851097
esp_transport_ws_send_raw(client->transport, WS_TRANSPORT_OPCODES_PONG | WS_TRANSPORT_OPCODES_FIN, data, client->payload_len,
10861098
client->config->network_timeout_ms);
1087-
#ifdef CONFIG_ESP_WS_CLIENT_SEPARATE_TX_LOCK
10881099
xSemaphoreGiveRecursive(client->tx_lock);
1100+
#else
1101+
// When separate TX lock is not configured, we already hold client->lock
1102+
// which protects the transport, so we can send PONG directly
1103+
esp_transport_ws_send_raw(client->transport, WS_TRANSPORT_OPCODES_PONG | WS_TRANSPORT_OPCODES_FIN, data, client->payload_len,
1104+
client->config->network_timeout_ms);
10891105
#endif
10901106
} else if (client->last_opcode == WS_TRANSPORT_OPCODES_PONG) {
10911107
client->wait_for_pong_resp = false;
@@ -1183,6 +1199,11 @@ static void esp_websocket_client_task(void *pv)
11831199
client->state = WEBSOCKET_STATE_CONNECTED;
11841200
client->wait_for_pong_resp = false;
11851201
client->error_handle.error_type = WEBSOCKET_ERROR_TYPE_NONE;
1202+
client->payload_len = 0;
1203+
client->payload_offset = 0;
1204+
client->last_fin = false;
1205+
client->last_opcode = WS_TRANSPORT_OPCODES_NONE;
1206+
11861207
esp_websocket_client_dispatch_event(client, WEBSOCKET_EVENT_CONNECTED, NULL, 0);
11871208
break;
11881209
case WEBSOCKET_STATE_CONNECTED:
@@ -1273,6 +1294,7 @@ static void esp_websocket_client_task(void *pv)
12731294
xSemaphoreTakeRecursive(client->lock, lock_timeout);
12741295
if (esp_websocket_client_recv(client) == ESP_FAIL) {
12751296
ESP_LOGE(TAG, "Error receive data");
1297+
// Note: Already holding client->lock from line above
12761298
esp_websocket_client_abort_connection(client, WEBSOCKET_ERROR_TYPE_TCP_TRANSPORT);
12771299
}
12781300
xSemaphoreGiveRecursive(client->lock);
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)