cleanup locking

esphome/core/scheduler.cpp

@@ -294,29 +294,27 @@ void HOT Scheduler::call() {
   }
 #endif  // ESPHOME_DEBUG_SCHEDULER
 
-  auto to_remove_was = this->to_remove_;
-  auto items_was = this->items_.size();
   // If we have too many items to remove
   if (this->to_remove_ > MAX_LOGICALLY_DELETED_ITEMS) {
+    // We hold the lock for the entire cleanup operation because:
+    // 1. We're rebuilding the entire items_ list, so we need exclusive access throughout
+    // 2. Other threads must see either the old state or the new state, not intermediate states
+    // 3. The operation is already expensive (O(n)), so lock overhead is negligible
+    // 4. No operations inside can block or take other locks, so no deadlock risk
+    LockGuard guard{this->lock_};
+
     std::vector<std::unique_ptr<SchedulerItem>> valid_items;
-    while (!this->empty_()) {
-      LockGuard guard{this->lock_};
-      auto item = std::move(this->items_[0]);
-      this->pop_raw_();
-      valid_items.push_back(std::move(item));
+
+    // Move all non-removed items to valid_items
+    for (auto &item : this->items_) {
+      if (!item->remove) {
+        valid_items.push_back(std::move(item));
+      }
     }
 
-    {
-      LockGuard guard{this->lock_};
-      this->items_ = std::move(valid_items);
-    }
-
-    // The following should not happen unless I'm missing something
-    if (this->to_remove_ != 0) {
-      ESP_LOGW(TAG, "to_remove_ was %" PRIu32 " now: %" PRIu32 " items where %zu now %zu. Please report this",
-               to_remove_was, to_remove_, items_was, items_.size());
-      this->to_remove_ = 0;
-    }
+    // Replace items_ with the filtered list
+    this->items_ = std::move(valid_items);
+    this->to_remove_ = 0;
   }
 
   while (!this->empty_()) {