Mirrors/esp-idf
1
0
Fork 0
mirror of https://github.com/espressif/esp-idf.git synced 2026-08-18 06:35:35 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(esp_http_server): take ctrl_sock_semaphore on shutdown and async wake

Browse Source 
httpd_stop() and httpd_req_async_handler_complete() both pushed
messages onto the control mbox via cs_send_to_ctrl_sock() without
reserving a slot in ctrl_sock_semaphore. Once the silent-drop fix
made the semaphore unconditional, the bypass became a real bug:
when the mbox is saturated by pending httpd_queue_work() items the
unguarded sendto() can return ENOBUFS, and even when it succeeds it
leaves the semaphore overstating free slots until the consumer
drains the message — a window during which a concurrent
httpd_queue_work() can take a slot but still find the mbox full.

Acquire the semaphore (portMAX_DELAY) before both sends and give it
back on send failure so the take/give invariant is preserved. The
httpd task is the consumer in both paths, so blocking is bounded
and deadlock-free. Reword the stale "no-op give on full" comment in
httpd_process_ctrl_msg() to reflect that only the recv-error path
relies on the cap behavior now.
This commit is contained in:
Ashish Sharma
2026-05-26 17:17:03 +08:00
parent fcc140c11c
commit dedb02dd25
2 changed files with 26 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
components/esp_http_server/src/httpd_main.c
View File

@@ -210,10 +210,10 @@ static void httpd_process_ctrl_msg(struct httpd_data *hd)
int ret = recv(hd->ctrl_fd, &msg, sizeof(msg), 0);
if (ret <= 0) {
ESP_LOGW(TAG, LOG_FMT("error in recv (%d)"), errno);
/* Returning the mbox slot to the producer side. xSemaphoreGive is a
* no-op once the counting semaphore is at its max, which keeps the
* accounting safe under producers that don't take (e.g. async
* handler wakeups, shutdown). */
/* No packet was actually consumed from the mbox here, so this give
* is unbalanced. It's tolerated because the counting semaphore is
* capped at its max — excess gives become no-ops. Spurious recv
* errors after select() are rare in practice. */
xSemaphoreGive(hd->ctrl_sock_semaphore);
return;
}
@@ -561,9 +561,19 @@ esp_err_t httpd_stop(httpd_handle_t handle)
struct httpd_ctrl_data msg;
memset(&msg, 0, sizeof(msg));
msg.hc_msg = HTTPD_CTRL_SHUTDOWN;
int ret = 0;
if ((ret = cs_send_to_ctrl_sock(hd->msg_fd, hd->config.ctrl_port, &msg, sizeof(msg))) < 0) {
/* Reserve a slot in the ctrl mbox before sending so we never push past
* its capacity. Blocking is safe: the httpd task is the consumer and
* keeps draining the mbox until it observes HTTPD_CTRL_SHUTDOWN. */
if (xSemaphoreTake(hd->ctrl_sock_semaphore, portMAX_DELAY) != pdTRUE) {
ESP_LOGE(TAG, "Failed to acquire ctrl socket semaphore");
return ESP_FAIL;
}
int ret = cs_send_to_ctrl_sock(hd->msg_fd, hd->config.ctrl_port, &msg, sizeof(msg));
if (ret < 0) {
ESP_LOGE(TAG, "Failed to send shutdown signal err=%d", ret);
xSemaphoreGive(hd->ctrl_sock_semaphore);
return ESP_FAIL;
}

10
components/esp_http_server/src/httpd_txrx.c
View File

@@ -730,9 +730,19 @@ esp_err_t httpd_req_async_handler_complete(httpd_req_t *r)
// will now re-add this FD to its select() descriptor list. This ensures that subsequent requests
// on the same FD are processed correctly
struct httpd_ctrl_data msg = {.hc_msg = HTTPD_CTRL_MAX};
/* Reserve an mbox slot so we don't overrun ctrl_sock_semaphore's
* accounting and starve concurrent httpd_queue_work() producers. The
* httpd main task is the consumer and will drain the mbox shortly. */
if (xSemaphoreTake(hd->ctrl_sock_semaphore, portMAX_DELAY) != pdTRUE) {
ESP_LOGW(TAG, LOG_FMT("failed to acquire ctrl socket semaphore"));
return ESP_FAIL;
}
int ret = cs_send_to_ctrl_sock(msg_fd, port, &msg, sizeof(msg));
if (ret < 0) {
ESP_LOGW(TAG, LOG_FMT("failed to send socket notification"));
xSemaphoreGive(hd->ctrl_sock_semaphore);
return ESP_FAIL;
}