From 3b6bd55d1e581422c0348daab7b97ce023b97b96 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 27 Jun 2025 13:16:06 -0500 Subject: [PATCH 1/2] address bot comments --- esphome/core/event_pool.h | 8 ++++++-- esphome/core/lock_free_queue.h | 15 +++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/esphome/core/event_pool.h b/esphome/core/event_pool.h index 6d61e9a80d..198c0fe380 100644 --- a/esphome/core/event_pool.h +++ b/esphome/core/event_pool.h @@ -1,6 +1,6 @@ #pragma once -#ifdef USE_ESP32 +#if defined(USE_ESP32) || defined(USE_LIBRETINY) #include #include @@ -17,6 +17,10 @@ template class EventPool { ~EventPool() { // Clean up any remaining events in the free list + // IMPORTANT: This destructor assumes no concurrent access. The EventPool must not + // be destroyed while any thread might still call allocate() or release(). + // In practice, this is typically ensured by destroying the pool only during + // component shutdown when all producer/consumer threads have been stopped. T *event; RAMAllocator allocator(RAMAllocator::ALLOC_INTERNAL); while ((event = this->free_list_.pop()) != nullptr) { @@ -72,4 +76,4 @@ template class EventPool { } // namespace esphome -#endif +#endif // defined(USE_ESP32) || defined(USE_LIBRETINY) diff --git a/esphome/core/lock_free_queue.h b/esphome/core/lock_free_queue.h index ede7496737..1fc5d25048 100644 --- a/esphome/core/lock_free_queue.h +++ b/esphome/core/lock_free_queue.h @@ -1,11 +1,17 @@ #pragma once -#ifdef USE_ESP32 +#if defined(USE_ESP32) || defined(USE_LIBRETINY) #include #include + +#if defined(USE_ESP32) #include #include +#elif defined(USE_LIBRETINY) +#include +#include +#endif /* * Lock-free queue for single-producer single-consumer scenarios. @@ -13,6 +19,8 @@ * blocking each other. * * This is a Single-Producer Single-Consumer (SPSC) lock-free ring buffer. + * Available on platforms with FreeRTOS support (ESP32, LibreTiny). + * * Common use cases: * - BLE events: BLE task produces, main loop consumes * - MQTT messages: main task produces, MQTT thread consumes @@ -56,6 +64,9 @@ template class LockFreeQueue { uint8_t head_after = head_.load(std::memory_order_acquire); if (head_after == current_tail) { // Consumer just caught up to where tail was - might go to sleep, must notify + // Note: There's a benign race here - between reading head_after and calling + // xTaskNotifyGive(), the consumer could advance further. This would result + // in an unnecessary wake-up, but is harmless and extremely rare in practice. xTaskNotifyGive(task_to_notify_); } // Otherwise: consumer is still behind, no need to notify @@ -113,4 +124,4 @@ template class LockFreeQueue { } // namespace esphome -#endif +#endif // defined(USE_ESP32) || defined(USE_LIBRETINY) From 95ef131285ee86b3ad09cfafa027ae9f7caab49c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 27 Jun 2025 13:18:39 -0500 Subject: [PATCH 2/2] address bot comments --- esphome/core/event_pool.h | 4 +++- esphome/core/lock_free_queue.h | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/esphome/core/event_pool.h b/esphome/core/event_pool.h index 198c0fe380..7a206f823e 100644 --- a/esphome/core/event_pool.h +++ b/esphome/core/event_pool.h @@ -11,6 +11,8 @@ namespace esphome { // Event Pool - On-demand pool of objects to avoid heap fragmentation // Events are allocated on first use and reused thereafter, growing to peak usage +// @tparam T The type of objects managed by the pool (must have a clear() method) +// @tparam SIZE The maximum number of objects in the pool (1-255, limited by uint8_t) template class EventPool { public: EventPool() : total_created_(0) {} @@ -71,7 +73,7 @@ template class EventPool { private: LockFreeQueue free_list_; // Free events ready for reuse - uint8_t total_created_; // Total events created (high water mark) + uint8_t total_created_; // Total events created (high water mark, max 255) }; } // namespace esphome diff --git a/esphome/core/lock_free_queue.h b/esphome/core/lock_free_queue.h index 1fc5d25048..5460be0fae 100644 --- a/esphome/core/lock_free_queue.h +++ b/esphome/core/lock_free_queue.h @@ -24,6 +24,9 @@ * Common use cases: * - BLE events: BLE task produces, main loop consumes * - MQTT messages: main task produces, MQTT thread consumes + * + * @tparam T The type of elements stored in the queue (must be a pointer type) + * @tparam SIZE The maximum number of elements (1-255, limited by uint8_t indices) */ namespace esphome { @@ -115,6 +118,8 @@ template class LockFreeQueue { // Atomic: written by producer (push/increment), read+reset by consumer (get_and_reset) std::atomic dropped_count_; // 65535 max - more than enough for drop tracking // Atomic: written by consumer (pop), read by producer (push) to check if full + // Using uint8_t limits queue size to 255 elements but saves memory and ensures + // atomic operations are efficient on all platforms std::atomic head_; // Atomic: written by producer (push), read by consumer (pop) to check if empty std::atomic tail_;