[core] Free the action/condition tree when an Automation is destroyed

esphome/components/sprinkler/sprinkler.cpp

@@ -431,13 +431,12 @@ void Sprinkler::add_valve(SprinklerControllerSwitch *valve_sw, SprinklerControll
 
   new_valve->valve_turn_off_automation =
       make_unique<Automation<>>(new_valve->controller_switch->get_turn_off_trigger());
-  new_valve->valve_shutdown_action = make_unique<sprinkler::ShutdownAction<>>(this);
-  new_valve->valve_turn_off_automation->add_actions({new_valve->valve_shutdown_action.get()});
+  new_valve->valve_turn_off_automation->add_actions({new sprinkler::ShutdownAction<>(this)});
 
   new_valve->valve_turn_on_automation = make_unique<Automation<>>(new_valve->controller_switch->get_turn_on_trigger());
-  new_valve->valve_resumeorstart_action = make_unique<sprinkler::StartSingleValveAction<>>(this);
-  new_valve->valve_resumeorstart_action->set_valve_to_start(new_valve_number);
-  new_valve->valve_turn_on_automation->add_actions({new_valve->valve_resumeorstart_action.get()});
+  auto *valve_resumeorstart_action = new sprinkler::StartSingleValveAction<>(this);
+  valve_resumeorstart_action->set_valve_to_start(new_valve_number);
+  new_valve->valve_turn_on_automation->add_actions({valve_resumeorstart_action});
 
   if (enable_sw != nullptr) {
     new_valve->enable_switch = enable_sw;
@@ -458,12 +457,10 @@ void Sprinkler::set_controller_main_switch(SprinklerControllerSwitch *controller
   });
 
   this->sprinkler_turn_off_automation_ = make_unique<Automation<>>(controller_switch->get_turn_off_trigger());
-  this->sprinkler_shutdown_action_ = make_unique<sprinkler::ShutdownAction<>>(this);
-  this->sprinkler_turn_off_automation_->add_actions({sprinkler_shutdown_action_.get()});
+  this->sprinkler_turn_off_automation_->add_actions({new sprinkler::ShutdownAction<>(this)});
 
   this->sprinkler_turn_on_automation_ = make_unique<Automation<>>(controller_switch->get_turn_on_trigger());
-  this->sprinkler_resumeorstart_action_ = make_unique<sprinkler::ResumeOrStartAction<>>(this);
-  this->sprinkler_turn_on_automation_->add_actions({sprinkler_resumeorstart_action_.get()});
+  this->sprinkler_turn_on_automation_->add_actions({new sprinkler::ResumeOrStartAction<>(this)});
 }
 
 void Sprinkler::set_controller_auto_adv_switch(SprinklerControllerSwitch *auto_adv_switch) {
@@ -482,8 +479,7 @@ void Sprinkler::set_controller_standby_switch(SprinklerControllerSwitch *standby
   this->standby_sw_ = standby_switch;
 
   this->sprinkler_standby_turn_on_automation_ = make_unique<Automation<>>(standby_switch->get_turn_on_trigger());
-  this->sprinkler_standby_shutdown_action_ = make_unique<sprinkler::ShutdownAction<>>(this);
-  this->sprinkler_standby_turn_on_automation_->add_actions({sprinkler_standby_shutdown_action_.get()});
+  this->sprinkler_standby_turn_on_automation_->add_actions({new sprinkler::ShutdownAction<>(this)});
 }
 
 void Sprinkler::set_controller_multiplier_number(SprinklerControllerNumber *multiplier_number) {

esphome/components/sprinkler/sprinkler.h

@@ -93,8 +93,7 @@ struct SprinklerValve {
   uint32_t run_duration;
   optional<size_t> pump_switch_index;
   bool valve_cycle_complete;
-  std::unique_ptr<ShutdownAction<>> valve_shutdown_action;
-  std::unique_ptr<StartSingleValveAction<>> valve_resumeorstart_action;
+  // The shutdown / start actions belong to the automations' ActionLists; a second owning handle would double-free.
   std::unique_ptr<Automation<>> valve_turn_off_automation;
   std::unique_ptr<Automation<>> valve_turn_on_automation;
 };
@@ -598,10 +597,7 @@ class Sprinkler : public Component {
   SprinklerControllerNumber *multiplier_number_{nullptr};
   SprinklerControllerNumber *repeat_number_{nullptr};
 
-  std::unique_ptr<ShutdownAction<>> sprinkler_shutdown_action_;
-  std::unique_ptr<ShutdownAction<>> sprinkler_standby_shutdown_action_;
-  std::unique_ptr<ResumeOrStartAction<>> sprinkler_resumeorstart_action_;
-
+  // As in SprinklerValve: the actions belong to the automations below.
   std::unique_ptr<Automation<>> sprinkler_turn_off_automation_;
   std::unique_ptr<Automation<>> sprinkler_turn_on_automation_;
   std::unique_ptr<Automation<>> sprinkler_standby_turn_on_automation_;

esphome/core/automation.h

@@ -123,6 +123,9 @@ template<typename T, typename... X> class TemplatableValue {
  */
 template<typename... Ts> class Condition {
  public:
+  /// Virtual so an owner can delete a child through this base.
+  virtual ~Condition() = default;
+
   /// Check whether this condition passes. This condition check must be instant, and not cause any delays.
   virtual bool check(Ts... x) = 0;
 
@@ -177,6 +180,9 @@ template<typename... Ts> class ActionList;
 
 template<typename... Ts> class Action {
  public:
+  /// Virtual so the owning ActionList can delete a node through this base.
+  virtual ~Action() = default;
+
   virtual void play_complex(Ts... x) {
     this->num_running_++;
     this->play(x...);
@@ -242,6 +248,23 @@ template<typename... Ts> class Action {
 
 template<typename... Ts> class ActionList {
  public:
+  ActionList() = default;
+
+  /// Owns its chain. Nothing reachable from a list that gets destroyed may be registered with App: Action and
+  /// Condition have no prepare_for_deletion() to decline with.
+  ~ActionList() {
+    Action<Ts...> *action = this->actions_begin_;
+    while (action != nullptr) {
+      Action<Ts...> *next = action->next_;
+      delete action;
+      action = next;
+    }
+  }
+
+  // Owning, so a copy would free the same chain twice.
+  ActionList(const ActionList &) = delete;
+  ActionList &operator=(const ActionList &) = delete;
+
   void add_action(Action<Ts...> *action) {
     if (this->actions_end_ == nullptr) {
       this->actions_begin_ = action;

esphome/core/base_automation.h

@@ -15,6 +15,11 @@ namespace esphome {
 template<typename... Ts> class AndCondition : public Condition<Ts...> {
  public:
   explicit AndCondition(const std::vector<Condition<Ts...> *> &conditions) : conditions_(conditions) {}
+  ~AndCondition() override {
+    for (auto *condition : this->conditions_) {
+      delete condition;
+    }
+  }
   bool check(Ts... x) override {
     for (auto *condition : this->conditions_) {
       if (!condition->check(x...))
@@ -31,6 +36,11 @@ template<typename... Ts> class AndCondition : public Condition<Ts...> {
 template<typename... Ts> class OrCondition : public Condition<Ts...> {
  public:
   explicit OrCondition(const std::vector<Condition<Ts...> *> &conditions) : conditions_(conditions) {}
+  ~OrCondition() override {
+    for (auto *condition : this->conditions_) {
+      delete condition;
+    }
+  }
   bool check(Ts... x) override {
     for (auto *condition : this->conditions_) {
       if (condition->check(x...))
@@ -47,6 +57,7 @@ template<typename... Ts> class OrCondition : public Condition<Ts...> {
 template<typename... Ts> class NotCondition : public Condition<Ts...> {
  public:
   explicit NotCondition(Condition<Ts...> *condition) : condition_(condition) {}
+  ~NotCondition() override { delete this->condition_; }
   bool check(Ts... x) override { return !this->condition_->check(x...); }
 
  protected:
@@ -56,6 +67,11 @@ template<typename... Ts> class NotCondition : public Condition<Ts...> {
 template<typename... Ts> class XorCondition : public Condition<Ts...> {
  public:
   explicit XorCondition(const std::vector<Condition<Ts...> *> &conditions) : conditions_(conditions) {}
+  ~XorCondition() override {
+    for (auto *condition : this->conditions_) {
+      delete condition;
+    }
+  }
   bool check(Ts... x) override {
     size_t result = 0;
     for (auto *condition : this->conditions_) {
@@ -81,6 +97,7 @@ template<typename... Ts> class LambdaCondition : public Condition<Ts...> {
 template<typename... Ts> class ForCondition : public Condition<Ts...>, public Component {
  public:
   explicit ForCondition(Condition<> *condition) : condition_(condition) {}
+  ~ForCondition() override { delete this->condition_; }
 
   TEMPLATABLE_VALUE(uint32_t, time);
 
@@ -167,6 +184,9 @@ template<typename... Ts> class DelayAction : public Action<Ts...>, public Compon
  public:
   explicit DelayAction() = default;
 
+  /// SchedulerItem keeps a raw Component*, and stop() does not run on every teardown path.
+  ~DelayAction() override { this->cancel_timeout("delay"); }
+
   TEMPLATABLE_VALUE(uint32_t, delay)
 
   void play_complex(Ts... x) override {
@@ -204,6 +224,8 @@ template<typename... Ts> class LambdaAction : public Action<Ts...> {
 template<typename... Ts> class IfAction : public Action<Ts...> {
  public:
   explicit IfAction(Condition<Ts...> *condition) : condition_(condition) {}
+  /// then_ / else_ are by-value ActionLists and free themselves.
+  ~IfAction() override { delete this->condition_; }
 
   void add_then(const std::vector<Action<Ts...> *> &actions) {
     this->then_.add_actions(actions);
@@ -250,6 +272,7 @@ template<typename... Ts> class IfAction : public Action<Ts...> {
 template<typename... Ts> class WhileAction : public Action<Ts...> {
  public:
   WhileAction(Condition<Ts...> *condition) : condition_(condition) {}
+  ~WhileAction() override { delete this->condition_; }
 
   void add_then(const std::vector<Action<Ts...> *> &actions) {
     this->then_.add_actions(actions);
@@ -333,6 +356,11 @@ template<typename... Ts> class RepeatAction : public Action<Ts...> {
 template<typename... Ts> class WaitUntilAction : public Action<Ts...>, public Component {
  public:
   WaitUntilAction(Condition<Ts...> *condition) : condition_(condition) {}
+  /// Same scheduler hazard as ~DelayAction, plus the owned condition.
+  ~WaitUntilAction() override {
+    this->cancel_timeout("timeout");
+    delete this->condition_;
+  }
 
   TEMPLATABLE_VALUE(uint32_t, timeout_value)
 

esphome/core/component.cpp

@@ -238,7 +238,7 @@ void IRAM_ATTR HOT Component::enable_loop_soon_any_context() {
   // 2. No read-modify-write operations that could be interrupted
   // 3. No memory allocation, object construction, or function calls
   // 4. IRAM_ATTR ensures code is in IRAM, not flash (required for ISR execution)
-  // 5. Components are never destroyed, so no use-after-free concerns
+  // 5. Components registered with App are never destroyed, so no use-after-free concerns
   // 6. App is guaranteed to be initialized before any ISR could fire
   // 7. Multiple ISR/thread calls are safe - just sets the same flags to true
   // 8. Race condition with main loop is handled by clearing flag before processing

esphome/core/scheduler.cpp

@@ -441,17 +441,12 @@ void HOT Scheduler::call(uint32_t now) {
       // Not reached timeout yet, done for this call
       break;
     }
-    // Don't run on failed components
-    if (item->component != nullptr && item->component->is_failed()) {
-      LockGuard guard{this->lock_};
-      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_()
+    // Must precede the is_failed() check below, as should_skip_item_() orders them: a merely marked item keeps a
+    // raw Component * that may already be freed.
 #ifdef ESPHOME_THREAD_MULTI_NO_ATOMICS
     // Multi-threaded platforms without atomics: must take lock to safely read remove flag
     {
@@ -472,6 +467,13 @@ void HOT Scheduler::call(uint32_t now) {
     }
 #endif
 
+    // Don't run on failed components
+    if (item->component != nullptr && item->component->is_failed()) {
+      LockGuard guard{this->lock_};
+      this->pop_raw_();
+      continue;
+    }
+
 #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 ")",