From 90c9cb98c6101180340ea0573022cb86c7c00965 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 16 Oct 2025 10:23:01 -1000 Subject: [PATCH] nullptr --- esphome/core/scheduler.cpp | 30 +++++++----------------------- esphome/core/scheduler.h | 30 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 2d22c1697f..ebd18fb57e 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -344,6 +344,12 @@ void HOT Scheduler::call(uint32_t now) { std::unique_ptr item; { LockGuard lock(this->lock_); + // 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_() at the end of this function + // 2. Any code iterating defer_queue_ MUST check for nullptr items (see mark_matching_items_removed_ + // and has_cancelled_timeout_in_container_ in scheduler.h) + // 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_++; } @@ -361,29 +367,7 @@ void HOT Scheduler::call(uint32_t now) { // Single consumer (main loop), so no lock needed for this check if (this->defer_queue_front_ >= defer_queue_end) { LockGuard lock(this->lock_); - // Check if new items were added by producers during processing - if (this->defer_queue_front_ >= this->defer_queue_.size()) { - // Common case: no new items - clear everything - this->defer_queue_.clear(); - } else { - // Rare case: new items were added during processing - compact the vector - // This only happens when: - // 1. A deferred callback calls defer() again, or - // 2. Another thread calls defer() while we're processing - // - // Move unprocessed items (added during this loop) to the front for next iteration - // - // SAFETY: Compacted items may include cancelled items (marked for removal via - // cancel_item_locked_() during execution). This is safe because should_skip_item_() - // checks is_item_removed_() before executing, so cancelled items will be skipped - // and recycled on the next loop iteration. - size_t remaining = this->defer_queue_.size() - this->defer_queue_front_; - for (size_t i = 0; i < remaining; i++) { - this->defer_queue_[i] = std::move(this->defer_queue_[this->defer_queue_front_ + i]); - } - this->defer_queue_.resize(remaining); - } - this->defer_queue_front_ = 0; + this->cleanup_defer_queue_locked_(); } #endif /* not ESPHOME_THREAD_SINGLE */ diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index b48c130d78..ad0ec0284e 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -264,6 +264,36 @@ class Scheduler { // Helper to recycle a SchedulerItem void recycle_item_(std::unique_ptr item); +#ifndef ESPHOME_THREAD_SINGLE + // Helper to cleanup defer_queue_ after processing + // IMPORTANT: Caller must hold the scheduler lock before calling this function. + inline void cleanup_defer_queue_locked_() { + // Check if new items were added by producers during processing + if (this->defer_queue_front_ >= this->defer_queue_.size()) { + // Common case: no new items - clear everything + this->defer_queue_.clear(); + } else { + // Rare case: new items were added during processing - compact the vector + // This only happens when: + // 1. A deferred callback calls defer() again, or + // 2. Another thread calls defer() while we're processing + // + // Move unprocessed items (added during this loop) to the front for next iteration + // + // SAFETY: Compacted items may include cancelled items (marked for removal via + // cancel_item_locked_() during execution). This is safe because should_skip_item_() + // checks is_item_removed_() before executing, so cancelled items will be skipped + // and recycled on the next loop iteration. + size_t remaining = this->defer_queue_.size() - this->defer_queue_front_; + for (size_t i = 0; i < remaining; i++) { + this->defer_queue_[i] = std::move(this->defer_queue_[this->defer_queue_front_ + i]); + } + this->defer_queue_.resize(remaining); + } + this->defer_queue_front_ = 0; + } +#endif /* not ESPHOME_THREAD_SINGLE */ + // Helper to check if item is marked for removal (platform-specific) // Returns true if item should be skipped, handles platform-specific synchronization // For ESPHOME_THREAD_MULTI_NO_ATOMICS platforms, the caller must hold the scheduler lock before calling this