From a4682615234ac61ff7825e2a922807b11e6772fa Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 19:41:26 -0600 Subject: [PATCH] [scheduler] De-template and consolidate scheduler helper functions (#14164) --- esphome/core/scheduler.cpp | 14 +++++++++---- esphome/core/scheduler.h | 41 +++++++------------------------------- 2 files changed, 17 insertions(+), 38 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 36b65f6ff7c..e4e0751e100 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -119,10 +119,16 @@ uint32_t Scheduler::calculate_interval_offset_(uint32_t delay) { // Remove before 2026.8.0 along with all retry code bool Scheduler::is_retry_cancelled_locked_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id) { - return has_cancelled_timeout_in_container_locked_(this->items_, component, name_type, static_name, hash_or_id, - /* match_retry= */ true) || - has_cancelled_timeout_in_container_locked_(this->to_add_, component, name_type, static_name, hash_or_id, - /* match_retry= */ true); + for (auto *container : {&this->items_, &this->to_add_}) { + for (auto &item : *container) { + if (item && this->is_item_removed_locked_(item.get()) && + this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, SchedulerItem::TIMEOUT, + /* match_retry= */ true, /* skip_removed= */ false)) { + return true; + } + } + } + return false; } // Common implementation for both timeout and interval diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 384d76b6b08..16b0ded3126 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -308,8 +308,8 @@ class Scheduler { SchedulerItem::Type type, bool match_retry, bool skip_removed = true) const { // THREAD SAFETY: Check for nullptr first to prevent LoadProhibited crashes. On multi-threaded // platforms, items can be moved out of defer_queue_ during processing, leaving nullptr entries. - // PR #11305 added nullptr checks in callers (mark_matching_items_removed_locked_() and - // has_cancelled_timeout_in_container_locked_()), but this check provides defense-in-depth: helper + // PR #11305 added nullptr checks in callers (mark_matching_items_removed_locked_()), but this check + // provides defense-in-depth: helper // functions should be safe regardless of caller behavior. // Fixes: https://github.com/esphome/esphome/issues/11940 if (!item) @@ -403,8 +403,7 @@ class Scheduler { // SAFETY: Moving out the unique_ptr leaves a nullptr in the vector at defer_queue_front_. // This is intentional and safe because: // 1. The vector is only cleaned up by cleanup_defer_queue_locked_() at the end of this function - // 2. Any code iterating defer_queue_ MUST check for nullptr items (see mark_matching_items_removed_locked_ - // and has_cancelled_timeout_in_container_locked_ in scheduler.h) + // 2. Any code iterating defer_queue_ MUST check for nullptr items (see mark_matching_items_removed_locked_) // 3. The lock protects concurrent access, but the nullptr remains until cleanup item = std::move(this->defer_queue_[this->defer_queue_front_]); this->defer_queue_front_++; @@ -497,19 +496,16 @@ class Scheduler { // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id // Returns the number of items marked for removal // IMPORTANT: Must be called with scheduler lock held - template - size_t mark_matching_items_removed_locked_(Container &container, Component *component, NameType name_type, - const char *static_name, uint32_t hash_or_id, SchedulerItem::Type type, - bool match_retry) { + size_t mark_matching_items_removed_locked_(std::vector> &container, + Component *component, NameType name_type, const char *static_name, + uint32_t hash_or_id, SchedulerItem::Type type, bool match_retry) { size_t count = 0; for (auto &item : container) { // Skip nullptr items (can happen in defer_queue_ when items are being processed) // The defer_queue_ uses index-based processing: items are std::moved out but left in the // vector as nullptr until cleanup. Even though this function is called with lock held, // the vector can still contain nullptr items from the processing loop. This check prevents crashes. - if (!item) - continue; - if (this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, type, match_retry)) { + if (item && this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, type, match_retry)) { this->set_item_removed_(item.get(), true); count++; } @@ -517,29 +513,6 @@ class Scheduler { return count; } - // Template helper to check if any item in a container matches our criteria - // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id - // IMPORTANT: Must be called with scheduler lock held - template - bool has_cancelled_timeout_in_container_locked_(const Container &container, Component *component, NameType name_type, - const char *static_name, uint32_t hash_or_id, - bool match_retry) const { - for (const auto &item : container) { - // Skip nullptr items (can happen in defer_queue_ when items are being processed) - // The defer_queue_ uses index-based processing: items are std::moved out but left in the - // vector as nullptr until cleanup. If this function is called during defer queue processing, - // it will iterate over these nullptr items. This check prevents crashes. - if (!item) - continue; - if (this->is_item_removed_locked_(item.get()) && - this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, SchedulerItem::TIMEOUT, - match_retry, /* skip_removed= */ false)) { - return true; - } - } - return false; - } - Mutex lock_; std::vector> items_; std::vector> to_add_;