Skip to content

Commit bff5cbe

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 bff5cbe

File tree

3 files changed

+46
-6
lines changed

3 files changed

+46
-6
lines changed

components/esp_websocket_client/esp_websocket_client.c

Lines changed: 29 additions & 6 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
@@ -1076,16 +1088,21 @@ static esp_err_t esp_websocket_client_recv(esp_websocket_client_handle_t client)
10761088

10771089
// CRITICAL: Check if transport is still valid after re-acquiring lock
10781090
// Another thread may have closed it while we didn't hold client->lock
1079-
if (client->state == WEBSOCKET_STATE_CLOSING || client->state == WEBSOCKET_STATE_UNKNOW || client->transport == NULL) {
1091+
if (client->state == WEBSOCKET_STATE_CLOSING || client->state == WEBSOCKET_STATE_UNKNOW ||
1092+
client->state == WEBSOCKET_STATE_WAIT_TIMEOUT || client->transport == NULL) {
10801093
ESP_LOGW(TAG, "Transport closed while preparing PONG, skipping send");
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);

components/esp_websocket_client/examples/target/main/idf_component.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,7 @@ dependencies:
44
espressif/esp_websocket_client:
55
version: "^1.0.0"
66
override_path: "../../../"
7+
espressif/cjson:
8+
version: "^1.7.15"
79
protocol_examples_common:
810
path: ${IDF_PATH}/examples/common_components/protocol_examples_common
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)