From 4ec2e42d4d2d8b50245b4ee603e1ed113471f3bb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 05:49:49 -0500 Subject: [PATCH] [scheduler] Rename counter mutators with _locked_ suffix Rename the seven counter RMW mutators to carry the `_locked_` suffix that matches the existing convention (pop_raw_locked_, is_item_removed_locked_, cancel_item_locked_, etc.): to_add_count_increment_ -> to_add_count_increment_locked_ to_add_count_clear_ -> to_add_count_clear_locked_ defer_count_increment_ -> defer_count_increment_locked_ defer_count_clear_ -> defer_count_clear_locked_ to_remove_add_ -> to_remove_add_locked_ to_remove_decrement_ -> to_remove_decrement_locked_ to_remove_clear_ -> to_remove_clear_locked_ The caller-must-hold-lock contract became load-bearing when the underlying counters became volatile on NO_ATOMICS: ++/+=/-- compile to a three-instruction LDR/OP/STR sequence that is not atomic against a concurrent RMW from another task, so the lock is what keeps the counter consistent. The new suffix makes the requirement explicit at every call site, matching how the rest of the scheduler documents the same invariant. No behavioural change; all call sites already hold lock_. --- esphome/core/scheduler.cpp | 20 ++++++++++---------- esphome/core/scheduler.h | 14 +++++++------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index b0eaa670ac..a6f1558e4a 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -235,11 +235,11 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type } target->push_back(item); if (target == &this->to_add_) { - this->to_add_count_increment_(); + this->to_add_count_increment_locked_(); } #ifndef ESPHOME_THREAD_SINGLE else { - this->defer_count_increment_(); + this->defer_count_increment_locked_(); } #endif } @@ -452,7 +452,7 @@ void Scheduler::full_cleanup_removed_items_() { this->items_.erase(this->items_.begin() + write, this->items_.end()); // Rebuild the heap structure since items are no longer in heap order std::make_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp); - this->to_remove_clear_(); + this->to_remove_clear_locked_(); } #ifndef ESPHOME_THREAD_SINGLE @@ -501,7 +501,7 @@ void HOT Scheduler::process_defer_queue_slow_path_(uint32_t &now) { this->lock_.lock(); // Reset counter and snapshot queue end under lock - this->defer_count_clear_(); + this->defer_count_clear_locked_(); size_t defer_queue_end = this->defer_queue_.size(); if (this->defer_queue_front_ >= defer_queue_end) { this->lock_.unlock(); @@ -621,7 +621,7 @@ uint32_t HOT Scheduler::call(uint32_t now) { LockGuard guard{this->lock_}; if (is_item_removed_locked_(item)) { this->recycle_item_main_loop_(this->pop_raw_locked_()); - this->to_remove_decrement_(); + this->to_remove_decrement_locked_(); continue; } } @@ -630,7 +630,7 @@ uint32_t HOT Scheduler::call(uint32_t now) { if (is_item_removed_(item)) { LockGuard guard{this->lock_}; this->recycle_item_main_loop_(this->pop_raw_locked_()); - this->to_remove_decrement_(); + this->to_remove_decrement_locked_(); continue; } #endif @@ -658,7 +658,7 @@ uint32_t HOT Scheduler::call(uint32_t now) { if (this->is_item_removed_locked_(executed_item)) { // We were removed/cancelled in the function call, recycle and continue - this->to_remove_decrement_(); + this->to_remove_decrement_locked_(); this->recycle_item_main_loop_(executed_item); continue; } @@ -721,7 +721,7 @@ void HOT Scheduler::process_to_add_slow_path_() { std::push_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp); } this->to_add_.clear(); - this->to_add_count_clear_(); + this->to_add_count_clear_locked_(); } bool HOT Scheduler::cleanup_slow_path_() { // We must hold the lock for the entire cleanup operation because: @@ -737,7 +737,7 @@ bool HOT Scheduler::cleanup_slow_path_() { SchedulerItem *item = this->items_[0]; if (!this->is_item_removed_locked_(item)) break; - this->to_remove_decrement_(); + this->to_remove_decrement_locked_(); this->recycle_item_main_loop_(this->pop_raw_locked_()); } return !this->items_.empty(); @@ -825,7 +825,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, NameType name_type size_t heap_cancelled = this->mark_matching_items_removed_locked_(this->items_, component, name_type, static_name, hash_or_id, type, match_retry, find_first); total_cancelled += heap_cancelled; - this->to_remove_add_(heap_cancelled); + this->to_remove_add_locked_(heap_cancelled); if (find_first && total_cancelled > 0) return true; } diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 096291426f..946ec8ca78 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -552,7 +552,7 @@ class Scheduler { } // Increment to_add_count_ (no-op on single-threaded platforms) - void to_add_count_increment_() { + void to_add_count_increment_locked_() { #ifdef ESPHOME_THREAD_SINGLE // No counter needed — to_add_empty_() checks the vector directly #elif defined(ESPHOME_THREAD_MULTI_ATOMICS) @@ -563,7 +563,7 @@ class Scheduler { } // Reset to_add_count_ (no-op on single-threaded platforms) - void to_add_count_clear_() { + void to_add_count_clear_locked_() { #ifdef ESPHOME_THREAD_SINGLE // No counter needed — to_add_empty_() checks the vector directly #elif defined(ESPHOME_THREAD_MULTI_ATOMICS) @@ -597,7 +597,7 @@ class Scheduler { #endif } - void defer_count_increment_() { + void defer_count_increment_locked_() { #ifdef ESPHOME_THREAD_MULTI_ATOMICS this->defer_count_.fetch_add(1, std::memory_order_relaxed); #else @@ -605,7 +605,7 @@ class Scheduler { #endif } - void defer_count_clear_() { + void defer_count_clear_locked_() { #ifdef ESPHOME_THREAD_MULTI_ATOMICS this->defer_count_.store(0, std::memory_order_relaxed); #else @@ -635,7 +635,7 @@ uint32_t to_remove_{0}; #endif } - void to_remove_add_(uint32_t count) { + void to_remove_add_locked_(uint32_t count) { #ifdef ESPHOME_THREAD_MULTI_ATOMICS this->to_remove_.fetch_add(count, std::memory_order_relaxed); #else @@ -643,7 +643,7 @@ uint32_t to_remove_{0}; #endif } - void to_remove_decrement_() { + void to_remove_decrement_locked_() { #ifdef ESPHOME_THREAD_MULTI_ATOMICS this->to_remove_.fetch_sub(1, std::memory_order_relaxed); #else @@ -651,7 +651,7 @@ uint32_t to_remove_{0}; #endif } - void to_remove_clear_() { + void to_remove_clear_locked_() { #ifdef ESPHOME_THREAD_MULTI_ATOMICS this->to_remove_.store(0, std::memory_order_relaxed); #else