diff --git a/esphome/components/rp2040_ble/rp2040_ble.cpp b/esphome/components/rp2040_ble/rp2040_ble.cpp index 405710f3e8..8e7c7d6be5 100644 --- a/esphome/components/rp2040_ble/rp2040_ble.cpp +++ b/esphome/components/rp2040_ble/rp2040_ble.cpp @@ -15,29 +15,18 @@ static const char *const TAG = "rp2040_ble"; // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) RP2040BLE *global_ble = nullptr; -// The analyzer cannot see that release() always retains the pointer here: the -// pool's free list is sized SIZE + 1, so its push cannot hit the ring-full -// drop branch for at most SIZE releases. -// NOLINTBEGIN(clang-analyzer-unix.Malloc) void RP2040BLE::setup() { global_ble = this; // Pre-create every pool entry so the packet handler's allocate() is always a - // free-list pop — the IRQ path must never reach malloc() (heap allocation - // after setup is forbidden, and the newlib malloc lock is not IRQ-safe). - // Deliberately unconditional: warming lazily on the first scan would move - // the allocations after setup, and doing it here keeps the pool's RAM cost - // visible at startup instead of appearing once scanning begins. - BLEScanReport *warm[MAX_SCAN_REPORT_QUEUE_SIZE - 1]; - size_t warmed = 0; - while (warmed < MAX_SCAN_REPORT_QUEUE_SIZE - 1 && (warm[warmed] = this->report_pool_.allocate()) != nullptr) - warmed++; - for (size_t i = 0; i < warmed; i++) - this->report_pool_.release(warm[i]); - if (warmed != MAX_SCAN_REPORT_QUEUE_SIZE - 1) { - // An incomplete warm would silently put malloc() back on the IRQ path once - // the free list runs dry; refuse to run instead (the stack is never - // enabled, so the packet handler cannot fire). + // free-list pop — the IRQ path must never reach malloc() (the newlib malloc + // lock is not IRQ-safe). Deliberately + // unconditional: warming lazily on the first scan would move the allocations + // after setup, and doing it here keeps the pool's RAM cost visible at + // startup instead of appearing once scanning begins. On an incomplete warm, + // refuse to run instead (the stack is never enabled, so the packet handler + // cannot fire). + if (!this->report_pool_.warm()) { ESP_LOGE(TAG, "Scan report pool warm-up failed"); this->mark_failed(); return; @@ -49,7 +38,6 @@ void RP2040BLE::setup() { this->state_ = BLEComponentState::DISABLED; } } -// NOLINTEND(clang-analyzer-unix.Malloc) void RP2040BLE::enable() { if (this->state_ == BLEComponentState::ACTIVE || this->state_ == BLEComponentState::ENABLING) { diff --git a/esphome/core/event_pool.h b/esphome/core/event_pool.h index fe207d04bf..b53b8064a3 100644 --- a/esphome/core/event_pool.h +++ b/esphome/core/event_pool.h @@ -10,7 +10,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 +// Events are allocated on first use and reused thereafter, growing to peak +// usage; warm() pre-creates every entry up front for malloc-free producers // @tparam T The type of objects managed by the pool (must have a release() method) // @tparam SIZE The maximum number of objects in the pool (1-254, limited by uint8_t and the +1 free-list slot) // @@ -53,26 +54,8 @@ template class EventPool { T *event = this->free_list_.pop(); if (event != nullptr) return event; - // Need to create a new event - if (this->total_created_ >= SIZE) { - // Pool is at capacity - return nullptr; - } - - // Use internal RAM for better performance - RAMAllocator allocator(RAMAllocator::ALLOC_INTERNAL); - event = allocator.allocate(1); - - if (event == nullptr) { - // Memory allocation failed - return nullptr; - } - - // Placement new to construct the object - new (event) T(); - this->total_created_++; - return event; + return this->create_(); } // Return an event to the pool for reuse @@ -84,7 +67,45 @@ template class EventPool { } } + // Pre-create every pool entry so allocate() is always a free-list pop + // (for producers that must never malloc, e.g. IRQ-context handlers). + // Call from setup(); on false the heap could not supply every entry and + // the caller should mark_failed() — an incomplete warm puts malloc() + // back on the producer path. Tops the pool up from any quiescent state + // (entries that already exist are counted, not re-created); must not run + // concurrently with allocate()/release(). + bool warm() { + // NOLINTNEXTLINE(clang-analyzer-unix.Malloc) -- ownership transfers to the free list + while (this->total_created_ < SIZE) { + T *event = this->create_(); + if (event == nullptr) + return false; + this->free_list_.push(event); + } + return true; + } + private: + // Create and count one new object (shared by allocate() and warm()). + // Returns nullptr at capacity or when the heap is exhausted. + T *create_() { + if (this->total_created_ >= SIZE) { + // Pool is at capacity + return nullptr; + } + // Use internal RAM for better performance + RAMAllocator allocator(RAMAllocator::ALLOC_INTERNAL); + T *event = allocator.allocate(1); + if (event == nullptr) { + // Memory allocation failed + return nullptr; + } + // Placement new to construct the object + new (event) T(); + this->total_created_++; + return event; + } + // SIZE + 1 slots so all SIZE objects fit when the pool is fully drained // (the ring reserves one slot); otherwise the last release() of a // completely returned pool would drop, permanently orphaning one object. diff --git a/tests/components/core/test_event_pool.cpp b/tests/components/core/test_event_pool.cpp index af54ac3e14..da13924c65 100644 --- a/tests/components/core/test_event_pool.cpp +++ b/tests/components/core/test_event_pool.cpp @@ -70,4 +70,67 @@ TEST(EventPool, ReleaseNullptrIsSafe) { EXPECT_NE(pool.allocate(), nullptr); } +TEST(EventPool, WarmFullyPopulatesThePool) { + // warm()'s guarantee is invisible at runtime: no later allocate() may touch + // malloc(). Fully populated means SIZE allocations succeed from the free + // list and the SIZE + 1-th refuses. + esphome::EventPool pool; + ASSERT_TRUE(pool.warm()); + PoolItem *items[4]; + for (auto *&item : items) { + item = pool.allocate(); + ASSERT_NE(item, nullptr); + } + EXPECT_EQ(pool.allocate(), nullptr); +} + +TEST(EventPool, WarmIsIdempotent) { + esphome::EventPool pool; + ASSERT_TRUE(pool.warm()); + ASSERT_TRUE(pool.warm()); + // Still exactly SIZE objects: no growth past capacity. + PoolItem *items[3]; + for (auto *&item : items) { + item = pool.allocate(); + ASSERT_NE(item, nullptr); + } + EXPECT_EQ(pool.allocate(), nullptr); +} + +TEST(EventPool, AllocateAfterWarmRecyclesTheWarmedObjects) { + // The objects handed out after warm() are the ones warm() created, + // recycled rather than re-created. + esphome::EventPool pool; + ASSERT_TRUE(pool.warm()); + std::set first_round; + PoolItem *items[4]; + for (auto *&item : items) { + item = pool.allocate(); + first_round.insert(item); + } + for (auto *item : items) + pool.release(item); + for (int i = 0; i < 4; i++) { + PoolItem *item = pool.allocate(); + ASSERT_NE(item, nullptr); + EXPECT_TRUE(first_round.count(item) == 1); + } +} + +TEST(EventPool, WarmTopsUpWithEntriesOutstanding) { + // warm() counts existing entries (free or checked out) instead of failing + // when some are outstanding: it tops the pool up from any state. + esphome::EventPool pool; + PoolItem *held = pool.allocate(); + ASSERT_NE(held, nullptr); + ASSERT_TRUE(pool.warm()); + // The held object plus three more accounts for all SIZE entries. + PoolItem *items[3]; + for (auto *&item : items) { + item = pool.allocate(); + ASSERT_NE(item, nullptr); + } + EXPECT_EQ(pool.allocate(), nullptr); +} + } // namespace esphome::core::testing