mirror of
https://github.com/esphome/esphome.git
synced 2026-05-23 11:16:52 +08:00
[scheduler] Replace unique_ptr with raw pointers, add leak detection (#14620)
This commit is contained in:
+112
-67
@@ -30,11 +30,6 @@ static constexpr uint32_t MAX_LOGICALLY_DELETED_ITEMS = 5;
|
||||
// max delay to start an interval sequence
|
||||
static constexpr uint32_t MAX_INTERVAL_DELAY = 5000;
|
||||
|
||||
// Prevent inlining of SchedulerItem deletion. On BK7231N (Thumb-1), GCC inlines
|
||||
// ~unique_ptr<SchedulerItem> (~30 bytes each) at every destruction site. Defining
|
||||
// the deleter in the .cpp file ensures a single copy of the destructor + operator delete.
|
||||
void Scheduler::SchedulerItemDeleter::operator()(SchedulerItem *ptr) const noexcept { delete ptr; }
|
||||
|
||||
#if defined(ESPHOME_LOG_HAS_VERBOSE) || defined(ESPHOME_DEBUG_SCHEDULER)
|
||||
// Helper struct for formatting scheduler item names consistently in logs
|
||||
// Uses a stack buffer to avoid heap allocation
|
||||
@@ -122,8 +117,8 @@ uint32_t Scheduler::calculate_interval_offset_(uint32_t delay) {
|
||||
bool Scheduler::is_retry_cancelled_locked_(Component *component, NameType name_type, const char *static_name,
|
||||
uint32_t hash_or_id) {
|
||||
for (auto *container : {&this->items_, &this->to_add_}) {
|
||||
for (auto &item : *container) {
|
||||
if (item && this->is_item_removed_locked_(item.get()) &&
|
||||
for (auto *item : *container) {
|
||||
if (item != nullptr && this->is_item_removed_locked_(item) &&
|
||||
this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, SchedulerItem::TIMEOUT,
|
||||
/* match_retry= */ true, /* skip_removed= */ false)) {
|
||||
return true;
|
||||
@@ -147,17 +142,31 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type
|
||||
return;
|
||||
}
|
||||
|
||||
// Take lock early to protect scheduler_item_pool_ access
|
||||
// Take lock early to protect scheduler_item_pool_ access and retry-cancelled check
|
||||
LockGuard guard{this->lock_};
|
||||
|
||||
// For retries, check if there's a cancelled timeout first - before allocating an item.
|
||||
// Skip check for anonymous retries (STATIC_STRING with nullptr) - they can't be cancelled by name
|
||||
// Skip check for defer (delay=0) - deferred retries bypass the cancellation check
|
||||
if (is_retry && delay != 0 && (name_type != NameType::STATIC_STRING || static_name != nullptr) &&
|
||||
type == SchedulerItem::TIMEOUT &&
|
||||
this->is_retry_cancelled_locked_(component, name_type, static_name, hash_or_id)) {
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
SchedulerNameLog skip_name_log;
|
||||
ESP_LOGD(TAG, "Skipping retry '%s' - found cancelled item",
|
||||
skip_name_log.format(name_type, static_name, hash_or_id));
|
||||
#endif
|
||||
return;
|
||||
}
|
||||
|
||||
// Create and populate the scheduler item
|
||||
auto item = this->get_item_from_pool_locked_();
|
||||
SchedulerItem *item = this->get_item_from_pool_locked_();
|
||||
item->component = component;
|
||||
item->set_name(name_type, static_name, hash_or_id);
|
||||
item->type = type;
|
||||
item->callback = std::move(func);
|
||||
// Reset remove flag - recycled items may have been cancelled (remove=true) in previous use
|
||||
this->set_item_removed_(item.get(), false);
|
||||
this->set_item_removed_(item, false);
|
||||
item->is_retry = is_retry;
|
||||
|
||||
// Determine target container: defer_queue_ for deferred items, to_add_ for everything else.
|
||||
@@ -193,29 +202,15 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type
|
||||
}
|
||||
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
this->debug_log_timer_(item.get(), name_type, static_name, hash_or_id, type, delay, now_64);
|
||||
this->debug_log_timer_(item, name_type, static_name, hash_or_id, type, delay, now_64);
|
||||
#endif /* ESPHOME_DEBUG_SCHEDULER */
|
||||
|
||||
// For retries, check if there's a cancelled timeout first
|
||||
// Skip check for anonymous retries (STATIC_STRING with nullptr) - they can't be cancelled by name
|
||||
if (is_retry && (name_type != NameType::STATIC_STRING || static_name != nullptr) &&
|
||||
type == SchedulerItem::TIMEOUT &&
|
||||
this->is_retry_cancelled_locked_(component, name_type, static_name, hash_or_id)) {
|
||||
// Skip scheduling - the retry was cancelled
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
SchedulerNameLog skip_name_log;
|
||||
ESP_LOGD(TAG, "Skipping retry '%s' - found cancelled item",
|
||||
skip_name_log.format(name_type, static_name, hash_or_id));
|
||||
#endif
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Common epilogue: atomic cancel-and-add (unless skip_cancel is true)
|
||||
if (!skip_cancel) {
|
||||
this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type);
|
||||
}
|
||||
target->push_back(std::move(item));
|
||||
target->push_back(item);
|
||||
}
|
||||
|
||||
void HOT Scheduler::set_timeout(Component *component, const char *name, uint32_t timeout,
|
||||
@@ -395,7 +390,7 @@ optional<uint32_t> HOT Scheduler::next_schedule_in(uint32_t now) {
|
||||
if (this->cleanup_() == 0)
|
||||
return {};
|
||||
|
||||
auto &item = this->items_[0];
|
||||
SchedulerItem *item = this->items_[0];
|
||||
const auto now_64 = this->millis_64_from_(now);
|
||||
const uint64_t next_exec = item->get_next_execution();
|
||||
if (next_exec < now_64)
|
||||
@@ -414,13 +409,13 @@ void Scheduler::full_cleanup_removed_items_() {
|
||||
// Compact in-place: move valid items forward, recycle removed ones
|
||||
size_t write = 0;
|
||||
for (size_t read = 0; read < this->items_.size(); ++read) {
|
||||
if (!is_item_removed_locked_(this->items_[read].get())) {
|
||||
if (!is_item_removed_locked_(this->items_[read])) {
|
||||
if (write != read) {
|
||||
this->items_[write] = std::move(this->items_[read]);
|
||||
this->items_[write] = this->items_[read];
|
||||
}
|
||||
++write;
|
||||
} else {
|
||||
this->recycle_item_main_loop_(std::move(this->items_[read]));
|
||||
this->recycle_item_main_loop_(this->items_[read]);
|
||||
}
|
||||
}
|
||||
this->items_.erase(this->items_.begin() + write, this->items_.end());
|
||||
@@ -444,7 +439,7 @@ void Scheduler::compact_defer_queue_locked_() {
|
||||
// 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_[i] = this->defer_queue_[this->defer_queue_front_ + i];
|
||||
}
|
||||
// Use erase() instead of resize() to avoid instantiating _M_default_append
|
||||
// (saves ~156 bytes flash). Erasing from the end is O(1) - no shifting needed.
|
||||
@@ -469,26 +464,26 @@ void HOT Scheduler::call(uint32_t now) {
|
||||
|
||||
if (now_64 - last_print > 2000) {
|
||||
last_print = now_64;
|
||||
std::vector<SchedulerItemPtr> old_items;
|
||||
std::vector<SchedulerItem *> old_items;
|
||||
ESP_LOGD(TAG, "Items: count=%zu, pool=%zu, now=%" PRIu64, this->items_.size(), this->scheduler_item_pool_.size(),
|
||||
now_64);
|
||||
// Cleanup before debug output
|
||||
this->cleanup_();
|
||||
while (!this->items_.empty()) {
|
||||
SchedulerItemPtr item;
|
||||
SchedulerItem *item;
|
||||
{
|
||||
LockGuard guard{this->lock_};
|
||||
item = this->pop_raw_locked_();
|
||||
}
|
||||
|
||||
SchedulerNameLog name_log;
|
||||
bool is_cancelled = is_item_removed_(item.get());
|
||||
bool is_cancelled = is_item_removed_(item);
|
||||
ESP_LOGD(TAG, " %s '%s/%s' interval=%" PRIu32 " next_execution in %" PRIu64 "ms at %" PRIu64 "%s",
|
||||
item->get_type_str(), LOG_STR_ARG(item->get_source()),
|
||||
name_log.format(item->get_name_type(), item->get_name(), item->get_name_hash_or_id()), item->interval,
|
||||
item->get_next_execution() - now_64, item->get_next_execution(), is_cancelled ? " [CANCELLED]" : "");
|
||||
|
||||
old_items.push_back(std::move(item));
|
||||
old_items.push_back(item);
|
||||
}
|
||||
ESP_LOGD(TAG, "\n");
|
||||
|
||||
@@ -512,7 +507,7 @@ void HOT Scheduler::call(uint32_t now) {
|
||||
}
|
||||
while (!this->items_.empty()) {
|
||||
// Don't copy-by value yet
|
||||
auto &item = this->items_[0];
|
||||
SchedulerItem *item = this->items_[0];
|
||||
if (item->get_next_execution() > now_64) {
|
||||
// Not reached timeout yet, done for this call
|
||||
break;
|
||||
@@ -532,7 +527,7 @@ void HOT Scheduler::call(uint32_t now) {
|
||||
// Multi-threaded platforms without atomics: must take lock to safely read remove flag
|
||||
{
|
||||
LockGuard guard{this->lock_};
|
||||
if (is_item_removed_locked_(item.get())) {
|
||||
if (is_item_removed_locked_(item)) {
|
||||
this->recycle_item_main_loop_(this->pop_raw_locked_());
|
||||
this->to_remove_--;
|
||||
continue;
|
||||
@@ -540,7 +535,7 @@ void HOT Scheduler::call(uint32_t now) {
|
||||
}
|
||||
#else
|
||||
// Single-threaded or multi-threaded with atomics: can check without lock
|
||||
if (is_item_removed_(item.get())) {
|
||||
if (is_item_removed_(item)) {
|
||||
LockGuard guard{this->lock_};
|
||||
this->recycle_item_main_loop_(this->pop_raw_locked_());
|
||||
this->to_remove_--;
|
||||
@@ -561,18 +556,18 @@ void HOT Scheduler::call(uint32_t now) {
|
||||
// Warning: During callback(), a lot of stuff can happen, including:
|
||||
// - timeouts/intervals get added, potentially invalidating vector pointers
|
||||
// - timeouts/intervals get cancelled
|
||||
now = this->execute_item_(item.get(), now);
|
||||
now = this->execute_item_(item, now);
|
||||
|
||||
LockGuard guard{this->lock_};
|
||||
|
||||
// Only pop after function call, this ensures we were reachable
|
||||
// during the function call and know if we were cancelled.
|
||||
auto executed_item = this->pop_raw_locked_();
|
||||
SchedulerItem *executed_item = this->pop_raw_locked_();
|
||||
|
||||
if (this->is_item_removed_locked_(executed_item.get())) {
|
||||
if (this->is_item_removed_locked_(executed_item)) {
|
||||
// We were removed/cancelled in the function call, recycle and continue
|
||||
this->to_remove_--;
|
||||
this->recycle_item_main_loop_(std::move(executed_item));
|
||||
this->recycle_item_main_loop_(executed_item);
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -580,10 +575,10 @@ void HOT Scheduler::call(uint32_t now) {
|
||||
executed_item->set_next_execution(now_64 + executed_item->interval);
|
||||
// Add new item directly to to_add_
|
||||
// since we have the lock held
|
||||
this->to_add_.push_back(std::move(executed_item));
|
||||
this->to_add_.push_back(executed_item);
|
||||
} else {
|
||||
// Timeout completed - recycle it
|
||||
this->recycle_item_main_loop_(std::move(executed_item));
|
||||
this->recycle_item_main_loop_(executed_item);
|
||||
}
|
||||
|
||||
has_added_items |= !this->to_add_.empty();
|
||||
@@ -592,17 +587,33 @@ void HOT Scheduler::call(uint32_t now) {
|
||||
if (has_added_items) {
|
||||
this->process_to_add();
|
||||
}
|
||||
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
// Verify no items were leaked during this call() cycle.
|
||||
// All items must be in items_, to_add_, defer_queue_, or the pool.
|
||||
// Safe to check here because:
|
||||
// - process_defer_queue_ has already run its cleanup_defer_queue_locked_(),
|
||||
// so defer_queue_ contains no nullptr slots inflating the count.
|
||||
// - The while loop above has finished, so no items are held in local variables;
|
||||
// every item has been returned to a container (items_, to_add_, or pool).
|
||||
// Lock needed to get a consistent snapshot of all containers.
|
||||
{
|
||||
LockGuard guard{this->lock_};
|
||||
this->debug_verify_no_leak_();
|
||||
}
|
||||
#endif
|
||||
}
|
||||
void HOT Scheduler::process_to_add() {
|
||||
LockGuard guard{this->lock_};
|
||||
for (auto &it : this->to_add_) {
|
||||
if (is_item_removed_locked_(it.get())) {
|
||||
for (auto *&it : this->to_add_) {
|
||||
if (is_item_removed_locked_(it)) {
|
||||
// Recycle cancelled items
|
||||
this->recycle_item_main_loop_(std::move(it));
|
||||
this->recycle_item_main_loop_(it);
|
||||
it = nullptr;
|
||||
continue;
|
||||
}
|
||||
|
||||
this->items_.push_back(std::move(it));
|
||||
this->items_.push_back(it);
|
||||
std::push_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp);
|
||||
}
|
||||
this->to_add_.clear();
|
||||
@@ -628,20 +639,18 @@ size_t HOT Scheduler::cleanup_() {
|
||||
// leading to race conditions
|
||||
LockGuard guard{this->lock_};
|
||||
while (!this->items_.empty()) {
|
||||
auto &item = this->items_[0];
|
||||
if (!this->is_item_removed_locked_(item.get()))
|
||||
SchedulerItem *item = this->items_[0];
|
||||
if (!this->is_item_removed_locked_(item))
|
||||
break;
|
||||
this->to_remove_--;
|
||||
this->recycle_item_main_loop_(this->pop_raw_locked_());
|
||||
}
|
||||
return this->items_.size();
|
||||
}
|
||||
Scheduler::SchedulerItemPtr HOT Scheduler::pop_raw_locked_() {
|
||||
Scheduler::SchedulerItem *HOT Scheduler::pop_raw_locked_() {
|
||||
std::pop_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp);
|
||||
|
||||
// Move the item out before popping - this is the item that was at the front of the heap
|
||||
auto item = std::move(this->items_.back());
|
||||
|
||||
SchedulerItem *item = this->items_.back();
|
||||
this->items_.pop_back();
|
||||
return item;
|
||||
}
|
||||
@@ -699,7 +708,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, NameType name_type
|
||||
return total_cancelled > 0;
|
||||
}
|
||||
|
||||
bool HOT Scheduler::SchedulerItem::cmp(const SchedulerItemPtr &a, const SchedulerItemPtr &b) {
|
||||
bool HOT Scheduler::SchedulerItem::cmp(SchedulerItem *a, SchedulerItem *b) {
|
||||
// High bits are almost always equal (change only on 32-bit rollover ~49 days)
|
||||
// Optimize for common case: check low bits first when high bits are equal
|
||||
return (a->next_execution_high_ == b->next_execution_high_) ? (a->next_execution_low_ > b->next_execution_low_)
|
||||
@@ -710,23 +719,26 @@ bool HOT Scheduler::SchedulerItem::cmp(const SchedulerItemPtr &a, const Schedule
|
||||
// IMPORTANT: Caller must hold the scheduler lock before calling this function.
|
||||
// This protects scheduler_item_pool_ from concurrent access by other threads
|
||||
// that may be acquiring items from the pool in set_timer_common_().
|
||||
void Scheduler::recycle_item_main_loop_(SchedulerItemPtr item) {
|
||||
if (!item)
|
||||
void Scheduler::recycle_item_main_loop_(SchedulerItem *item) {
|
||||
if (item == nullptr)
|
||||
return;
|
||||
|
||||
if (this->scheduler_item_pool_.size() < MAX_POOL_SIZE) {
|
||||
// Clear callback to release captured resources
|
||||
item->callback = nullptr;
|
||||
this->scheduler_item_pool_.push_back(std::move(item));
|
||||
this->scheduler_item_pool_.push_back(item);
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
ESP_LOGD(TAG, "Recycled item to pool (pool size now: %zu)", this->scheduler_item_pool_.size());
|
||||
#endif
|
||||
} else {
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
ESP_LOGD(TAG, "Pool full (size: %zu), deleting item", this->scheduler_item_pool_.size());
|
||||
#endif
|
||||
delete item;
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
this->debug_live_items_--;
|
||||
#endif
|
||||
}
|
||||
// else: unique_ptr will delete the item when it goes out of scope
|
||||
}
|
||||
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
@@ -753,21 +765,54 @@ void Scheduler::debug_log_timer_(const SchedulerItem *item, NameType name_type,
|
||||
|
||||
// Helper to get or create a scheduler item from the pool
|
||||
// IMPORTANT: Caller must hold the scheduler lock before calling this function.
|
||||
Scheduler::SchedulerItemPtr Scheduler::get_item_from_pool_locked_() {
|
||||
SchedulerItemPtr item;
|
||||
Scheduler::SchedulerItem *Scheduler::get_item_from_pool_locked_() {
|
||||
if (!this->scheduler_item_pool_.empty()) {
|
||||
item = std::move(this->scheduler_item_pool_.back());
|
||||
SchedulerItem *item = this->scheduler_item_pool_.back();
|
||||
this->scheduler_item_pool_.pop_back();
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
ESP_LOGD(TAG, "Reused item from pool (pool size now: %zu)", this->scheduler_item_pool_.size());
|
||||
#endif
|
||||
} else {
|
||||
item = SchedulerItemPtr(new SchedulerItem());
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
ESP_LOGD(TAG, "Allocated new item (pool empty)");
|
||||
#endif
|
||||
return item;
|
||||
}
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
ESP_LOGD(TAG, "Allocated new item (pool empty)");
|
||||
#endif
|
||||
auto *item = new SchedulerItem();
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
this->debug_live_items_++;
|
||||
#endif
|
||||
return item;
|
||||
}
|
||||
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
bool Scheduler::debug_verify_no_leak_() const {
|
||||
// Invariant: every live SchedulerItem must be in exactly one container.
|
||||
// debug_live_items_ tracks allocations minus deletions.
|
||||
size_t accounted = this->items_.size() + this->to_add_.size() + this->scheduler_item_pool_.size();
|
||||
#ifndef ESPHOME_THREAD_SINGLE
|
||||
accounted += this->defer_queue_.size();
|
||||
#endif
|
||||
if (accounted != this->debug_live_items_) {
|
||||
ESP_LOGE(TAG,
|
||||
"SCHEDULER LEAK DETECTED: live=%" PRIu32 " but accounted=%" PRIu32 " (items=%" PRIu32 " to_add=%" PRIu32
|
||||
" pool=%" PRIu32
|
||||
#ifndef ESPHOME_THREAD_SINGLE
|
||||
" defer=%" PRIu32
|
||||
#endif
|
||||
")",
|
||||
static_cast<uint32_t>(this->debug_live_items_), static_cast<uint32_t>(accounted),
|
||||
static_cast<uint32_t>(this->items_.size()), static_cast<uint32_t>(this->to_add_.size()),
|
||||
static_cast<uint32_t>(this->scheduler_item_pool_.size())
|
||||
#ifndef ESPHOME_THREAD_SINGLE
|
||||
,
|
||||
static_cast<uint32_t>(this->defer_queue_.size())
|
||||
#endif
|
||||
);
|
||||
assert(false);
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
#endif
|
||||
|
||||
} // namespace esphome
|
||||
|
||||
+44
-50
@@ -2,7 +2,6 @@
|
||||
|
||||
#include "esphome/core/defines.h"
|
||||
#include <cstring>
|
||||
#include <memory>
|
||||
#include <string>
|
||||
#include <vector>
|
||||
#ifdef ESPHOME_THREAD_MULTI_ATOMICS
|
||||
@@ -144,19 +143,6 @@ class Scheduler {
|
||||
};
|
||||
|
||||
protected:
|
||||
struct SchedulerItem;
|
||||
|
||||
// Custom deleter for SchedulerItem unique_ptr that prevents the compiler from
|
||||
// inlining the destructor at every destruction site. On BK7231N (Thumb-1), GCC
|
||||
// inlines ~unique_ptr<SchedulerItem> (~30 bytes: null check + ~std::function +
|
||||
// operator delete) at every destruction site, while ESP32/ESP8266/RTL8720CF outline
|
||||
// it into a single helper. This noinline deleter ensures only one copy exists.
|
||||
// operator() is defined in scheduler.cpp to prevent inlining.
|
||||
struct SchedulerItemDeleter {
|
||||
void operator()(SchedulerItem *ptr) const noexcept;
|
||||
};
|
||||
using SchedulerItemPtr = std::unique_ptr<SchedulerItem, SchedulerItemDeleter>;
|
||||
|
||||
struct SchedulerItem {
|
||||
// Ordered by size to minimize padding
|
||||
Component *component;
|
||||
@@ -219,14 +205,14 @@ class Scheduler {
|
||||
name_.static_name = nullptr;
|
||||
}
|
||||
|
||||
// Destructor - no dynamic memory to clean up
|
||||
// Destructor - no dynamic memory to clean up (callback's std::function handles its own)
|
||||
~SchedulerItem() = default;
|
||||
|
||||
// Delete copy operations to prevent accidental copies
|
||||
SchedulerItem(const SchedulerItem &) = delete;
|
||||
SchedulerItem &operator=(const SchedulerItem &) = delete;
|
||||
|
||||
// Delete move operations: SchedulerItem objects are only managed via unique_ptr, never moved directly
|
||||
// Delete move operations: SchedulerItem objects are managed via raw pointers, never moved directly
|
||||
SchedulerItem(SchedulerItem &&) = delete;
|
||||
SchedulerItem &operator=(SchedulerItem &&) = delete;
|
||||
|
||||
@@ -250,7 +236,7 @@ class Scheduler {
|
||||
name_type_ = type;
|
||||
}
|
||||
|
||||
static bool cmp(const SchedulerItemPtr &a, const SchedulerItemPtr &b);
|
||||
static bool cmp(SchedulerItem *a, SchedulerItem *b);
|
||||
|
||||
// Note: We use 48 bits total (32 + 16), stored in a 64-bit value for API compatibility.
|
||||
// The upper 16 bits of the 64-bit value are always zero, which is fine since
|
||||
@@ -301,12 +287,13 @@ class Scheduler {
|
||||
// Returns the number of items remaining after cleanup
|
||||
// IMPORTANT: This method should only be called from the main thread (loop task).
|
||||
size_t cleanup_();
|
||||
// Remove and return the front item from the heap
|
||||
// Remove and return the front item from the heap as a raw pointer.
|
||||
// Caller takes ownership and must either recycle or delete the item.
|
||||
// IMPORTANT: Caller must hold the scheduler lock before calling this function.
|
||||
SchedulerItemPtr pop_raw_locked_();
|
||||
SchedulerItem *pop_raw_locked_();
|
||||
// Get or create a scheduler item from the pool
|
||||
// IMPORTANT: Caller must hold the scheduler lock before calling this function.
|
||||
SchedulerItemPtr get_item_from_pool_locked_();
|
||||
SchedulerItem *get_item_from_pool_locked_();
|
||||
|
||||
private:
|
||||
// Helper to cancel items - must be called with lock held
|
||||
@@ -330,19 +317,16 @@ class Scheduler {
|
||||
// Helper function to check if item matches criteria for cancellation
|
||||
// name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id
|
||||
// IMPORTANT: Must be called with scheduler lock held
|
||||
inline bool HOT matches_item_locked_(const SchedulerItemPtr &item, Component *component, NameType name_type,
|
||||
inline bool HOT matches_item_locked_(SchedulerItem *item, Component *component, NameType name_type,
|
||||
const char *static_name, uint32_t hash_or_id, SchedulerItem::Type type,
|
||||
bool match_retry, bool skip_removed = true) const {
|
||||
// THREAD SAFETY: Check for nullptr first to prevent LoadProhibited crashes. On multi-threaded
|
||||
// platforms, items can be moved out of defer_queue_ during processing, leaving nullptr entries.
|
||||
// PR #11305 added nullptr checks in callers (mark_matching_items_removed_locked_()), but this check
|
||||
// provides defense-in-depth: helper
|
||||
// functions should be safe regardless of caller behavior.
|
||||
// platforms, items can be nulled in defer_queue_ during processing.
|
||||
// Fixes: https://github.com/esphome/esphome/issues/11940
|
||||
if (!item)
|
||||
if (item == nullptr)
|
||||
return false;
|
||||
if (item->component != component || item->type != type ||
|
||||
(skip_removed && this->is_item_removed_locked_(item.get())) || (match_retry && !item->is_retry)) {
|
||||
if (item->component != component || item->type != type || (skip_removed && this->is_item_removed_locked_(item)) ||
|
||||
(match_retry && !item->is_retry)) {
|
||||
return false;
|
||||
}
|
||||
// Name type must match
|
||||
@@ -364,10 +348,12 @@ class Scheduler {
|
||||
}
|
||||
|
||||
// Helper to recycle a SchedulerItem back to the pool.
|
||||
// Takes a raw pointer — caller transfers ownership. The item is either added to the
|
||||
// pool or deleted if the pool is full.
|
||||
// IMPORTANT: Only call from main loop context! Recycling clears the callback,
|
||||
// so calling from another thread while the callback is executing causes use-after-free.
|
||||
// IMPORTANT: Caller must hold the scheduler lock before calling this function.
|
||||
void recycle_item_main_loop_(SchedulerItemPtr item);
|
||||
void recycle_item_main_loop_(SchedulerItem *item);
|
||||
|
||||
// Helper to perform full cleanup when too many items are cancelled
|
||||
void full_cleanup_removed_items_();
|
||||
@@ -423,27 +409,28 @@ class Scheduler {
|
||||
// Merge lock acquisitions: instead of separate locks for move-out and recycle (2N+1 total),
|
||||
// recycle each item after re-acquiring the lock for the next iteration (N+1 total).
|
||||
// The lock is held across: recycle → loop condition → move-out, then released for execution.
|
||||
SchedulerItemPtr item;
|
||||
SchedulerItem *item;
|
||||
|
||||
this->lock_.lock();
|
||||
while (this->defer_queue_front_ < defer_queue_end) {
|
||||
// SAFETY: Moving out the unique_ptr leaves a nullptr in the vector at defer_queue_front_.
|
||||
// This is intentional and safe because:
|
||||
// Take ownership of the item, leaving nullptr in the vector slot.
|
||||
// This is safe because:
|
||||
// 1. The vector is only cleaned up by cleanup_defer_queue_locked_() at the end of this function
|
||||
// 2. Any code iterating defer_queue_ MUST check for nullptr items (see mark_matching_items_removed_locked_)
|
||||
// 3. The lock protects concurrent access, but the nullptr remains until cleanup
|
||||
item = std::move(this->defer_queue_[this->defer_queue_front_]);
|
||||
item = this->defer_queue_[this->defer_queue_front_];
|
||||
this->defer_queue_[this->defer_queue_front_] = nullptr;
|
||||
this->defer_queue_front_++;
|
||||
this->lock_.unlock();
|
||||
|
||||
// Execute callback without holding lock to prevent deadlocks
|
||||
// if the callback tries to call defer() again
|
||||
if (!this->should_skip_item_(item.get())) {
|
||||
now = this->execute_item_(item.get(), now);
|
||||
if (!this->should_skip_item_(item)) {
|
||||
now = this->execute_item_(item, now);
|
||||
}
|
||||
|
||||
this->lock_.lock();
|
||||
this->recycle_item_main_loop_(std::move(item));
|
||||
this->recycle_item_main_loop_(item);
|
||||
}
|
||||
// Clean up the queue (lock already held from last recycle or initial acquisition)
|
||||
this->cleanup_defer_queue_locked_();
|
||||
@@ -523,18 +510,14 @@ class Scheduler {
|
||||
// name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id
|
||||
// Returns the number of items marked for removal
|
||||
// IMPORTANT: Must be called with scheduler lock held
|
||||
__attribute__((noinline)) size_t mark_matching_items_removed_locked_(std::vector<SchedulerItemPtr> &container,
|
||||
__attribute__((noinline)) size_t mark_matching_items_removed_locked_(std::vector<SchedulerItem *> &container,
|
||||
Component *component, NameType name_type,
|
||||
const char *static_name, uint32_t hash_or_id,
|
||||
SchedulerItem::Type type, bool match_retry) {
|
||||
size_t count = 0;
|
||||
for (auto &item : container) {
|
||||
// Skip nullptr items (can happen in defer_queue_ when items are being processed)
|
||||
// The defer_queue_ uses index-based processing: items are std::moved out but left in the
|
||||
// vector as nullptr until cleanup. Even though this function is called with lock held,
|
||||
// the vector can still contain nullptr items from the processing loop. This check prevents crashes.
|
||||
if (item && this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, type, match_retry)) {
|
||||
this->set_item_removed_(item.get(), true);
|
||||
for (auto *item : container) {
|
||||
if (this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, type, match_retry)) {
|
||||
this->set_item_removed_(item, true);
|
||||
count++;
|
||||
}
|
||||
}
|
||||
@@ -542,15 +525,15 @@ class Scheduler {
|
||||
}
|
||||
|
||||
Mutex lock_;
|
||||
std::vector<SchedulerItemPtr> items_;
|
||||
std::vector<SchedulerItemPtr> to_add_;
|
||||
std::vector<SchedulerItem *> items_;
|
||||
std::vector<SchedulerItem *> to_add_;
|
||||
#ifndef ESPHOME_THREAD_SINGLE
|
||||
// Single-core platforms don't need the defer queue and save ~32 bytes of RAM
|
||||
// Using std::vector instead of std::deque avoids 512-byte chunked allocations
|
||||
// Index tracking avoids O(n) erase() calls when draining the queue each loop
|
||||
std::vector<SchedulerItemPtr> defer_queue_; // FIFO queue for defer() calls
|
||||
size_t defer_queue_front_{0}; // Index of first valid item in defer_queue_ (tracks consumed items)
|
||||
#endif /* ESPHOME_THREAD_SINGLE */
|
||||
std::vector<SchedulerItem *> defer_queue_; // FIFO queue for defer() calls
|
||||
size_t defer_queue_front_{0}; // Index of first valid item in defer_queue_ (tracks consumed items)
|
||||
#endif /* ESPHOME_THREAD_SINGLE */
|
||||
uint32_t to_remove_{0};
|
||||
|
||||
// Memory pool for recycling SchedulerItem objects to reduce heap churn.
|
||||
@@ -561,7 +544,18 @@ class Scheduler {
|
||||
// - The pool significantly reduces heap fragmentation which is critical because heap allocation/deallocation
|
||||
// can stall the entire system, causing timing issues and dropped events for any components that need
|
||||
// to synchronize between tasks (see https://github.com/esphome/backlog/issues/52)
|
||||
std::vector<SchedulerItemPtr> scheduler_item_pool_;
|
||||
std::vector<SchedulerItem *> scheduler_item_pool_;
|
||||
|
||||
#ifdef ESPHOME_DEBUG_SCHEDULER
|
||||
// Leak detection: tracks total live SchedulerItem allocations.
|
||||
// Invariant: debug_live_items_ == items_.size() + to_add_.size() + defer_queue_.size() + scheduler_item_pool_.size()
|
||||
// Verified periodically in call() to catch leaks early.
|
||||
size_t debug_live_items_{0};
|
||||
|
||||
// Verify the scheduler memory invariant: all allocated items are accounted for.
|
||||
// Returns true if no leak detected. Logs an error and asserts on failure.
|
||||
bool debug_verify_no_leak_() const;
|
||||
#endif
|
||||
};
|
||||
|
||||
} // namespace esphome
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: scheduler-bulk-cleanup
|
||||
|
||||
external_components:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: scheduler-defer-cancel
|
||||
|
||||
host:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: scheduler-defer-cancel-regular
|
||||
|
||||
host:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: scheduler-defer-fifo-simple
|
||||
|
||||
host:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: scheduler-defer-stress-test
|
||||
|
||||
external_components:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: scheduler-heap-stress-test
|
||||
|
||||
external_components:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: scheduler-internal-id-test
|
||||
on_boot:
|
||||
priority: -100
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: scheduler-null-name
|
||||
|
||||
host:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: scheduler-numeric-id-test
|
||||
on_boot:
|
||||
priority: -100
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: sched-rapid-cancel-test
|
||||
|
||||
external_components:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: sched-recursive-timeout
|
||||
|
||||
external_components:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: scheduler-removed-item-race
|
||||
|
||||
host:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: scheduler-retry-test
|
||||
on_boot:
|
||||
priority: -100
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: sched-simul-callbacks-test
|
||||
|
||||
external_components:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: scheduler-string-lifetime-test
|
||||
|
||||
external_components:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
esphome:
|
||||
debug_scheduler: true # Enable scheduler leak detection
|
||||
name: sched-string-name-stress
|
||||
|
||||
external_components:
|
||||
|
||||
Reference in New Issue
Block a user