mirror of
				https://github.com/esphome/esphome.git
				synced 2025-10-30 22:53:59 +00:00 
			
		
		
		
	Fix thread-safe cleanup of event source connections in ESP-IDF web server (#9268)
This commit is contained in:
		| @@ -292,22 +292,39 @@ void AsyncEventSource::handleRequest(AsyncWebServerRequest *request) { | |||||||
| } | } | ||||||
|  |  | ||||||
| void AsyncEventSource::loop() { | void AsyncEventSource::loop() { | ||||||
|   for (auto *ses : this->sessions_) { |   // Clean up dead sessions safely | ||||||
|  |   // This follows the ESP-IDF pattern where free_ctx marks resources as dead | ||||||
|  |   // and the main loop handles the actual cleanup to avoid race conditions | ||||||
|  |   auto it = this->sessions_.begin(); | ||||||
|  |   while (it != this->sessions_.end()) { | ||||||
|  |     auto *ses = *it; | ||||||
|  |     // If the session has a dead socket (marked by destroy callback) | ||||||
|  |     if (ses->fd_.load() == 0) { | ||||||
|  |       ESP_LOGD(TAG, "Removing dead event source session"); | ||||||
|  |       it = this->sessions_.erase(it); | ||||||
|  |       delete ses;  // NOLINT(cppcoreguidelines-owning-memory) | ||||||
|  |     } else { | ||||||
|       ses->loop(); |       ses->loop(); | ||||||
|  |       ++it; | ||||||
|  |     } | ||||||
|   } |   } | ||||||
| } | } | ||||||
|  |  | ||||||
| void AsyncEventSource::try_send_nodefer(const char *message, const char *event, uint32_t id, uint32_t reconnect) { | void AsyncEventSource::try_send_nodefer(const char *message, const char *event, uint32_t id, uint32_t reconnect) { | ||||||
|   for (auto *ses : this->sessions_) { |   for (auto *ses : this->sessions_) { | ||||||
|  |     if (ses->fd_.load() != 0) {  // Skip dead sessions | ||||||
|       ses->try_send_nodefer(message, event, id, reconnect); |       ses->try_send_nodefer(message, event, id, reconnect); | ||||||
|     } |     } | ||||||
|  |   } | ||||||
| } | } | ||||||
|  |  | ||||||
| void AsyncEventSource::deferrable_send_state(void *source, const char *event_type, | void AsyncEventSource::deferrable_send_state(void *source, const char *event_type, | ||||||
|                                              message_generator_t *message_generator) { |                                              message_generator_t *message_generator) { | ||||||
|   for (auto *ses : this->sessions_) { |   for (auto *ses : this->sessions_) { | ||||||
|  |     if (ses->fd_.load() != 0) {  // Skip dead sessions | ||||||
|       ses->deferrable_send_state(source, event_type, message_generator); |       ses->deferrable_send_state(source, event_type, message_generator); | ||||||
|     } |     } | ||||||
|  |   } | ||||||
| } | } | ||||||
|  |  | ||||||
| AsyncEventSourceResponse::AsyncEventSourceResponse(const AsyncWebServerRequest *request, | AsyncEventSourceResponse::AsyncEventSourceResponse(const AsyncWebServerRequest *request, | ||||||
| @@ -331,7 +348,7 @@ AsyncEventSourceResponse::AsyncEventSourceResponse(const AsyncWebServerRequest * | |||||||
|   req->free_ctx = AsyncEventSourceResponse::destroy; |   req->free_ctx = AsyncEventSourceResponse::destroy; | ||||||
|  |  | ||||||
|   this->hd_ = req->handle; |   this->hd_ = req->handle; | ||||||
|   this->fd_ = httpd_req_to_sockfd(req); |   this->fd_.store(httpd_req_to_sockfd(req)); | ||||||
|  |  | ||||||
|   // Configure reconnect timeout and send config |   // Configure reconnect timeout and send config | ||||||
|   // this should always go through since the tcp send buffer is empty on connect |   // this should always go through since the tcp send buffer is empty on connect | ||||||
| @@ -362,8 +379,10 @@ AsyncEventSourceResponse::AsyncEventSourceResponse(const AsyncWebServerRequest * | |||||||
|  |  | ||||||
| void AsyncEventSourceResponse::destroy(void *ptr) { | void AsyncEventSourceResponse::destroy(void *ptr) { | ||||||
|   auto *rsp = static_cast<AsyncEventSourceResponse *>(ptr); |   auto *rsp = static_cast<AsyncEventSourceResponse *>(ptr); | ||||||
|   rsp->server_->sessions_.erase(rsp); |   ESP_LOGD(TAG, "Event source connection closed (fd: %d)", rsp->fd_.load()); | ||||||
|   delete rsp;  // NOLINT(cppcoreguidelines-owning-memory) |   // Mark as dead by setting fd to 0 - will be cleaned up in the main loop | ||||||
|  |   rsp->fd_.store(0); | ||||||
|  |   // Note: We don't delete or remove from set here to avoid race conditions | ||||||
| } | } | ||||||
|  |  | ||||||
| // helper for allowing only unique entries in the queue | // helper for allowing only unique entries in the queue | ||||||
| @@ -403,9 +422,11 @@ void AsyncEventSourceResponse::process_buffer_() { | |||||||
|     return; |     return; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   int bytes_sent = httpd_socket_send(this->hd_, this->fd_, event_buffer_.c_str() + event_bytes_sent_, |   int bytes_sent = httpd_socket_send(this->hd_, this->fd_.load(), event_buffer_.c_str() + event_bytes_sent_, | ||||||
|                                      event_buffer_.size() - event_bytes_sent_, 0); |                                      event_buffer_.size() - event_bytes_sent_, 0); | ||||||
|   if (bytes_sent == HTTPD_SOCK_ERR_TIMEOUT || bytes_sent == HTTPD_SOCK_ERR_FAIL) { |   if (bytes_sent == HTTPD_SOCK_ERR_TIMEOUT || bytes_sent == HTTPD_SOCK_ERR_FAIL) { | ||||||
|  |     // Socket error - just return, the connection will be closed by httpd | ||||||
|  |     // and our destroy callback will be called | ||||||
|     return; |     return; | ||||||
|   } |   } | ||||||
|   event_bytes_sent_ += bytes_sent; |   event_bytes_sent_ += bytes_sent; | ||||||
| @@ -425,7 +446,7 @@ void AsyncEventSourceResponse::loop() { | |||||||
|  |  | ||||||
| bool AsyncEventSourceResponse::try_send_nodefer(const char *message, const char *event, uint32_t id, | bool AsyncEventSourceResponse::try_send_nodefer(const char *message, const char *event, uint32_t id, | ||||||
|                                                 uint32_t reconnect) { |                                                 uint32_t reconnect) { | ||||||
|   if (this->fd_ == 0) { |   if (this->fd_.load() == 0) { | ||||||
|     return false; |     return false; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   | |||||||
| @@ -4,6 +4,7 @@ | |||||||
| #include "esphome/core/defines.h" | #include "esphome/core/defines.h" | ||||||
| #include <esp_http_server.h> | #include <esp_http_server.h> | ||||||
|  |  | ||||||
|  | #include <atomic> | ||||||
| #include <functional> | #include <functional> | ||||||
| #include <list> | #include <list> | ||||||
| #include <map> | #include <map> | ||||||
| @@ -271,7 +272,7 @@ class AsyncEventSourceResponse { | |||||||
|   static void destroy(void *p); |   static void destroy(void *p); | ||||||
|   AsyncEventSource *server_; |   AsyncEventSource *server_; | ||||||
|   httpd_handle_t hd_{}; |   httpd_handle_t hd_{}; | ||||||
|   int fd_{}; |   std::atomic<int> fd_{}; | ||||||
|   std::vector<DeferredEvent> deferred_queue_; |   std::vector<DeferredEvent> deferred_queue_; | ||||||
|   esphome::web_server::WebServer *web_server_; |   esphome::web_server::WebServer *web_server_; | ||||||
|   std::unique_ptr<esphome::web_server::ListEntitiesIterator> entities_iterator_; |   std::unique_ptr<esphome::web_server::ListEntitiesIterator> entities_iterator_; | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user