diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index d6d99f82c8..9e66fd3432 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -219,10 +219,13 @@ bool HOT Scheduler::cancel_retry(Component *component, const std::string &name) optional HOT Scheduler::next_schedule_in(uint32_t now) { // IMPORTANT: This method should only be called from the main thread (loop task). - // It calls empty_() and accesses items_[0] without holding a lock, which is only + // It performs cleanup and accesses items_[0] without holding a lock, which is only // safe when called from the main thread. Other threads must not call this method. - if (this->empty_()) + + // If no items, return empty optional + if (this->cleanup_() == 0) return {}; + auto &item = this->items_[0]; // Convert the fresh timestamp from caller (usually Application::loop()) to 64-bit const auto now_64 = this->millis_64_(now); // 'now' from parameter - fresh from caller @@ -282,7 +285,9 @@ void HOT Scheduler::call(uint32_t now) { ESP_LOGD(TAG, "Items: count=%zu, now=%" PRIu64 " (%" PRIu16 ", %" PRIu32 ")", this->items_.size(), now_64, this->millis_major_, this->last_millis_); #endif /* else ESPHOME_CORES_MULTI_ATOMICS */ - while (!this->empty_()) { + // Cleanup before debug output + this->cleanup_(); + while (!this->items_.empty()) { std::unique_ptr item; { LockGuard guard{this->lock_}; @@ -333,7 +338,9 @@ void HOT Scheduler::call(uint32_t now) { this->to_remove_ = 0; } - while (!this->empty_()) { + // Cleanup removed items before processing + this->cleanup_(); + while (!this->items_.empty()) { // use scoping to indicate visibility of `item` variable { // Don't copy-by value yet @@ -399,8 +406,8 @@ void HOT Scheduler::process_to_add() { } this->to_add_.clear(); } -void HOT Scheduler::cleanup_() { - // Fast path: if nothing to remove, just return +size_t HOT Scheduler::cleanup_() { + // Fast path: if nothing to remove, just return the current size // Reading to_remove_ without lock is safe because: // 1. We only call this from the main thread during call() // 2. If it's 0, there's definitely nothing to cleanup @@ -408,7 +415,7 @@ void HOT Scheduler::cleanup_() { // 4. Not all platforms support atomics, so we accept this race in favor of performance // 5. The worst case is a one-loop-iteration delay in cleanup, which is harmless if (this->to_remove_ == 0) - return; + return this->items_.size(); // We must hold the lock for the entire cleanup operation because: // 1. We're modifying items_ (via pop_raw_) which requires exclusive access @@ -422,10 +429,11 @@ void HOT Scheduler::cleanup_() { while (!this->items_.empty()) { auto &item = this->items_[0]; if (!item->remove) - return; + break; this->to_remove_--; this->pop_raw_(); } + return this->items_.size(); } void HOT Scheduler::pop_raw_() { std::pop_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp); diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index b539b26949..c14b7debe4 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -58,6 +58,9 @@ class Scheduler { // Calculate when the next scheduled item should run // @param now Fresh timestamp from millis() - must not be stale/cached + // Returns the time in milliseconds until the next scheduled item, or nullopt if no items + // This method performs cleanup of removed items before checking the schedule + // IMPORTANT: This method should only be called from the main thread (loop task). optional next_schedule_in(uint32_t now); // Execute all scheduled items that are ready @@ -147,7 +150,10 @@ class Scheduler { uint32_t delay, std::function func); uint64_t millis_64_(uint32_t now); - void cleanup_(); + // Cleanup logically deleted items from the scheduler + // Returns the number of items remaining after cleanup + // IMPORTANT: This method should only be called from the main thread (loop task). + size_t cleanup_(); void pop_raw_(); private: @@ -191,17 +197,6 @@ class Scheduler { return item->remove || (item->component != nullptr && item->component->is_failed()); } - // Check if the scheduler has no items. - // IMPORTANT: This method should only be called from the main thread (loop task). - // It performs cleanup of removed items and checks if the queue is empty. - // The items_.empty() check at the end is done without a lock for performance, - // which is safe because this is only called from the main thread while other - // threads only add items (never remove them). - bool empty_() { - this->cleanup_(); - return this->items_.empty(); - } - Mutex lock_; std::vector> items_; std::vector> to_add_;