mirror of
				https://github.com/esphome/esphome.git
				synced 2025-10-31 15:12:06 +00:00 
			
		
		
		
	fix another race
This commit is contained in:
		| @@ -383,19 +383,29 @@ void HOT Scheduler::process_to_add() { | |||||||
|   this->to_add_.clear(); |   this->to_add_.clear(); | ||||||
| } | } | ||||||
| void HOT Scheduler::cleanup_() { | void HOT Scheduler::cleanup_() { | ||||||
|  |   // Fast path: if nothing to remove, just return | ||||||
|  |   // Reading to_remove_ without lock is safe because: | ||||||
|  |   // 1. It's volatile, ensuring we read the latest value | ||||||
|  |   // 2. If it's 0, there's definitely nothing to cleanup | ||||||
|  |   // 3. If it becomes non-zero after we check, cleanup will happen next time | ||||||
|  |   if (this->to_remove_ == 0) | ||||||
|  |     return; | ||||||
|  |  | ||||||
|  |   // We must hold the lock for the entire cleanup operation because: | ||||||
|  |   // 1. We're modifying items_ (via pop_raw_) which other threads may be reading/writing | ||||||
|  |   // 2. We're decrementing to_remove_ which must be synchronized with increments | ||||||
|  |   // 3. We need a consistent view of items_ throughout the iteration | ||||||
|  |   // 4. Other threads might be adding items or modifying the heap structure | ||||||
|  |   // Without the lock, we could have race conditions leading to crashes or corruption | ||||||
|  |   LockGuard guard{this->lock_}; | ||||||
|   while (!this->items_.empty()) { |   while (!this->items_.empty()) { | ||||||
|     auto &item = this->items_[0]; |     auto &item = this->items_[0]; | ||||||
|     if (!item->remove) |     if (!item->remove) | ||||||
|       return; |       return; | ||||||
|  |  | ||||||
|     this->to_remove_--; |     this->to_remove_--; | ||||||
|  |  | ||||||
|     { |  | ||||||
|       LockGuard guard{this->lock_}; |  | ||||||
|     this->pop_raw_(); |     this->pop_raw_(); | ||||||
|   } |   } | ||||||
| } | } | ||||||
| } |  | ||||||
| void HOT Scheduler::pop_raw_() { | void HOT Scheduler::pop_raw_() { | ||||||
|   std::pop_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp); |   std::pop_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp); | ||||||
|   this->items_.pop_back(); |   this->items_.pop_back(); | ||||||
|   | |||||||
| @@ -185,6 +185,12 @@ class Scheduler { | |||||||
|     return item->remove || (item->component != nullptr && item->component->is_failed()); |     return item->remove || (item->component != nullptr && item->component->is_failed()); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   // Check if the scheduler has no items. | ||||||
|  |   // IMPORTANT: This method should only be called from the main thread (loop task). | ||||||
|  |   // It performs cleanup of removed items and checks if the queue is empty. | ||||||
|  |   // The items_.empty() check at the end is done without a lock for performance, | ||||||
|  |   // which is safe because this is only called from the main thread while other | ||||||
|  |   // threads only add items (never remove them). | ||||||
|   bool empty_() { |   bool empty_() { | ||||||
|     this->cleanup_(); |     this->cleanup_(); | ||||||
|     return this->items_.empty(); |     return this->items_.empty(); | ||||||
| @@ -202,7 +208,7 @@ class Scheduler { | |||||||
| #endif | #endif | ||||||
|   uint32_t last_millis_{0}; |   uint32_t last_millis_{0}; | ||||||
|   uint16_t millis_major_{0}; |   uint16_t millis_major_{0}; | ||||||
|   uint32_t to_remove_{0}; |   volatile uint32_t to_remove_{0}; | ||||||
| }; | }; | ||||||
|  |  | ||||||
| }  // namespace esphome | }  // namespace esphome | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user