atomic remove

esphome/core/scheduler.cpp

@@ -82,7 +82,13 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type
   item->set_name(name_cstr, !is_static_string);
   item->type = type;
   item->callback = std::move(func);
+  // Initialize remove to false (though it should already be from constructor)
+  // Not using mark_item_removed_ helper since we're setting to false, not true
+#ifdef ESPHOME_THREAD_MULTI_ATOMICS
+  item->remove.store(false, std::memory_order_relaxed);
+#else
   item->remove = false;
+#endif
   item->is_retry = is_retry;
 
 #ifndef ESPHOME_THREAD_SINGLE
@@ -398,6 +404,31 @@ void HOT Scheduler::call(uint32_t now) {
         this->pop_raw_();
         continue;
       }
+
+      // Check if item is marked for removal
+      // This handles two cases:
+      // 1. Item was marked for removal after cleanup_() but before we got here
+      // 2. Item is marked for removal but wasn't at the front of the heap during cleanup_()
+#ifdef ESPHOME_THREAD_MULTI_NO_ATOMICS
+      // Multi-threaded platforms without atomics: must take lock to safely read remove flag
+      {
+        LockGuard guard{this->lock_};
+        if (is_item_removed_(item.get())) {
+          this->pop_raw_();
+          this->to_remove_--;
+          continue;
+        }
+      }
+#else
+      // Single-threaded or multi-threaded with atomics: can check without lock
+      if (is_item_removed_(item.get())) {
+        LockGuard guard{this->lock_};
+        this->pop_raw_();
+        this->to_remove_--;
+        continue;
+      }
+#endif
+
 #ifdef ESPHOME_DEBUG_SCHEDULER
       const char *item_name = item->get_name();
       ESP_LOGV(TAG, "Running %s '%s/%s' with interval=%" PRIu32 " next_execution=%" PRIu64 " (now=%" PRIu64 ")",
@@ -518,7 +549,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c
   if (type == SchedulerItem::TIMEOUT) {
     for (auto &item : this->defer_queue_) {
       if (this->matches_item_(item, component, name_cstr, type, match_retry)) {
-        item->remove = true;
+        this->mark_item_removed_(item.get());
         total_cancelled++;
       }
     }
@@ -528,7 +559,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c
   // Cancel items in the main heap
   for (auto &item : this->items_) {
     if (this->matches_item_(item, component, name_cstr, type, match_retry)) {
-      item->remove = true;
+      this->mark_item_removed_(item.get());
       total_cancelled++;
       this->to_remove_++;  // Track removals for heap items
     }
@@ -537,7 +568,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c
   // Cancel items in to_add_
   for (auto &item : this->to_add_) {
     if (this->matches_item_(item, component, name_cstr, type, match_retry)) {
-      item->remove = true;
+      this->mark_item_removed_(item.get());
       total_cancelled++;
       // Don't track removals for to_add_ items
     }

esphome/core/scheduler.h

@@ -97,22 +97,42 @@ class Scheduler {
 
     std::function<void()> callback;
 
-    // Bit-packed fields to minimize padding
+#ifdef ESPHOME_THREAD_MULTI_ATOMICS
+    // Multi-threaded with atomics: use atomic for lock-free access
+    // Place atomic<bool> separately since it can't be packed with bit fields
+    std::atomic<bool> remove{false};
+
+    // Bit-packed fields (3 bits used, 5 bits padding in 1 byte)
+    enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1;
+    bool name_is_dynamic : 1;  // True if name was dynamically allocated (needs delete[])
+    bool is_retry : 1;         // True if this is a retry timeout
+                               // 5 bits padding
+#else
+    // Single-threaded or multi-threaded without atomics: can pack all fields together
+    // Bit-packed fields (4 bits used, 4 bits padding in 1 byte)
     enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1;
     bool remove : 1;
     bool name_is_dynamic : 1;  // True if name was dynamically allocated (needs delete[])
     bool is_retry : 1;         // True if this is a retry timeout
-    // 4 bits padding
+                               // 4 bits padding
+#endif
 
     // Constructor
     SchedulerItem()
         : component(nullptr),
           interval(0),
           next_execution_(0),
+#ifdef ESPHOME_THREAD_MULTI_ATOMICS
+          // remove is initialized in the member declaration as std::atomic<bool>{false}
+          type(TIMEOUT),
+          name_is_dynamic(false),
+          is_retry(false) {
+#else
           type(TIMEOUT),
           remove(false),
           name_is_dynamic(false),
           is_retry(false) {
+#endif
       name_.static_name = nullptr;
     }
 
@@ -219,6 +239,35 @@ class Scheduler {
     return item->remove || (item->component != nullptr && item->component->is_failed());
   }
 
+  // Helper to check if item is marked for removal (platform-specific)
+  // Returns true if item should be skipped, handles platform-specific synchronization
+  // NOTE: For ESPHOME_THREAD_MULTI_NO_ATOMICS, caller must hold lock!
+  bool is_item_removed_(SchedulerItem *item) const {
+#ifdef ESPHOME_THREAD_MULTI_ATOMICS
+    // Multi-threaded with atomics: use atomic load for lock-free access
+    return item->remove.load(std::memory_order_acquire);
+#else
+    // Single-threaded (ESPHOME_THREAD_SINGLE) or
+    // multi-threaded without atomics (ESPHOME_THREAD_MULTI_NO_ATOMICS): direct read
+    // For ESPHOME_THREAD_MULTI_NO_ATOMICS, caller MUST hold lock!
+    return item->remove;
+#endif
+  }
+
+  // Helper to mark item for removal (platform-specific)
+  // NOTE: For ESPHOME_THREAD_MULTI_NO_ATOMICS, caller must hold lock!
+  void mark_item_removed_(SchedulerItem *item) {
+#ifdef ESPHOME_THREAD_MULTI_ATOMICS
+    // Multi-threaded with atomics: use atomic store
+    item->remove.store(true, std::memory_order_release);
+#else
+    // Single-threaded (ESPHOME_THREAD_SINGLE) or
+    // multi-threaded without atomics (ESPHOME_THREAD_MULTI_NO_ATOMICS): direct write
+    // For ESPHOME_THREAD_MULTI_NO_ATOMICS, caller MUST hold lock!
+    item->remove = true;
+#endif
+  }
+
   // Template helper to check if any item in a container matches our criteria
   template<typename Container>
   bool has_cancelled_timeout_in_container_(const Container &container, Component *component, const char *name_cstr,