1
0
mirror of https://github.com/esphome/esphome.git synced 2025-10-30 14:43:51 +00:00

[core] Fix scheduler race condition where cancelled items still execute (#10268)

This commit is contained in:
J. Nick Koston
2025-08-18 11:14:41 -04:00
committed by GitHub
parent 0a77423073
commit 6818439109
4 changed files with 328 additions and 5 deletions

View File

@@ -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,37 @@ 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
// For ESPHOME_THREAD_MULTI_NO_ATOMICS platforms, the caller must hold the scheduler lock before calling this
// function.
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)
// For ESPHOME_THREAD_MULTI_NO_ATOMICS platforms, the caller must hold the scheduler lock before calling this
// function.
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,