Add enable_loop_soon_any_context() for thread and ISR-safe loop enabling (#9127)

This commit is contained in:
J. Nick Koston
2025-06-19 13:30:41 +12:00
committed by GitHub
parent 2e11e66db4
commit 8ba22183b9
9 changed files with 325 additions and 20 deletions
+83 -11
View File
@@ -97,6 +97,20 @@ void Application::loop() {
// Feed WDT with time
this->feed_wdt(last_op_end_time);
// Process any pending enable_loop requests from ISRs
// This must be done before marking in_loop_ = true to avoid race conditions
if (this->has_pending_enable_loop_requests_) {
// Clear flag BEFORE processing to avoid race condition
// If ISR sets it during processing, we'll catch it next loop iteration
// This is safe because:
// 1. Each component has its own pending_enable_loop_ flag that we check
// 2. If we can't process a component (wrong state), enable_pending_loops_()
// will set this flag back to true
// 3. Any new ISR requests during processing will set the flag again
this->has_pending_enable_loop_requests_ = false;
this->enable_pending_loops_();
}
// Mark that we're in the loop for safe reentrant modifications
this->in_loop_ = true;
@@ -286,24 +300,82 @@ void Application::disable_component_loop_(Component *component) {
}
}
void Application::activate_looping_component_(uint16_t index) {
// Helper to move component from inactive to active section
if (index != this->looping_components_active_end_) {
std::swap(this->looping_components_[index], this->looping_components_[this->looping_components_active_end_]);
}
this->looping_components_active_end_++;
}
void Application::enable_component_loop_(Component *component) {
// This method must be reentrant - components can re-enable themselves during their own loop() call
// Single pass through all components to find and move if needed
// With typical 10-30 components, O(n) is faster than maintaining a map
// This method is only called when component state is LOOP_DONE, so we know
// the component must be in the inactive section (if it exists in looping_components_)
// Only search the inactive portion for better performance
// With typical 0-5 inactive components, O(k) is much faster than O(n)
const uint16_t size = this->looping_components_.size();
for (uint16_t i = 0; i < size; i++) {
for (uint16_t i = this->looping_components_active_end_; i < size; i++) {
if (this->looping_components_[i] == component) {
if (i < this->looping_components_active_end_) {
return; // Already active
}
// Found in inactive section - move to active
if (i != this->looping_components_active_end_) {
std::swap(this->looping_components_[i], this->looping_components_[this->looping_components_active_end_]);
}
this->looping_components_active_end_++;
this->activate_looping_component_(i);
return;
}
}
// Component not found in looping_components_ - this is normal for components
// that don't have loop() or were not included in the partitioned vector
}
void Application::enable_pending_loops_() {
// Process components that requested enable_loop from ISR context
// Only iterate through inactive looping_components_ (typically 0-5) instead of all components
//
// Race condition handling:
// 1. We check if component is already in LOOP state first - if so, just clear the flag
// This handles reentrancy where enable_loop() was called between ISR and processing
// 2. We only clear pending_enable_loop_ after checking state, preventing lost requests
// 3. If any components aren't in LOOP_DONE state, we set has_pending_enable_loop_requests_
// back to true to ensure we check again next iteration
// 4. ISRs can safely set flags at any time - worst case is we process them next iteration
// 5. The global flag (has_pending_enable_loop_requests_) is cleared before this method,
// so any ISR that fires during processing will be caught in the next loop
const uint16_t size = this->looping_components_.size();
bool has_pending = false;
for (uint16_t i = this->looping_components_active_end_; i < size; i++) {
Component *component = this->looping_components_[i];
if (!component->pending_enable_loop_) {
continue; // Skip components without pending requests
}
// Check current state
uint8_t state = component->component_state_ & COMPONENT_STATE_MASK;
// If already in LOOP state, nothing to do - clear flag and continue
if (state == COMPONENT_STATE_LOOP) {
component->pending_enable_loop_ = false;
continue;
}
// If not in LOOP_DONE state, can't enable yet - keep flag set
if (state != COMPONENT_STATE_LOOP_DONE) {
has_pending = true; // Keep tracking this component
continue; // Keep the flag set - try again next iteration
}
// Clear the pending flag and enable the loop
component->pending_enable_loop_ = false;
ESP_LOGD(TAG, "%s loop enabled from ISR", component->get_component_source());
component->component_state_ &= ~COMPONENT_STATE_MASK;
component->component_state_ |= COMPONENT_STATE_LOOP;
// Move to active section
this->activate_looping_component_(i);
}
// If we couldn't process some requests, ensure we check again next iteration
if (has_pending) {
this->has_pending_enable_loop_requests_ = true;
}
}
#ifdef USE_SOCKET_SELECT_SUPPORT
+3
View File
@@ -577,6 +577,8 @@ class Application {
// to ensure component state is properly updated along with the loop partition
void disable_component_loop_(Component *component);
void enable_component_loop_(Component *component);
void enable_pending_loops_();
void activate_looping_component_(uint16_t index);
void feed_wdt_arch_();
@@ -682,6 +684,7 @@ class Application {
uint32_t loop_interval_{16};
size_t dump_config_at_{SIZE_MAX};
uint8_t app_state_{0};
volatile bool has_pending_enable_loop_requests_{false};
Component *current_component_{nullptr};
uint32_t loop_component_start_time_{0};
+19 -4
View File
@@ -148,10 +148,12 @@ void Component::mark_failed() {
App.disable_component_loop_(this);
}
void Component::disable_loop() {
ESP_LOGD(TAG, "%s loop disabled", this->get_component_source());
this->component_state_ &= ~COMPONENT_STATE_MASK;
this->component_state_ |= COMPONENT_STATE_LOOP_DONE;
App.disable_component_loop_(this);
if ((this->component_state_ & COMPONENT_STATE_MASK) != COMPONENT_STATE_LOOP_DONE) {
ESP_LOGD(TAG, "%s loop disabled", this->get_component_source());
this->component_state_ &= ~COMPONENT_STATE_MASK;
this->component_state_ |= COMPONENT_STATE_LOOP_DONE;
App.disable_component_loop_(this);
}
}
void Component::enable_loop() {
if ((this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_LOOP_DONE) {
@@ -161,6 +163,19 @@ void Component::enable_loop() {
App.enable_component_loop_(this);
}
}
void IRAM_ATTR HOT Component::enable_loop_soon_any_context() {
// This method is thread and ISR-safe because:
// 1. Only performs simple assignments to volatile variables (atomic on all platforms)
// 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
// 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
this->pending_enable_loop_ = true;
App.has_pending_enable_loop_requests_ = true;
}
void Component::reset_to_construction_state() {
if ((this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_FAILED) {
ESP_LOGI(TAG, "Component %s is being reset to construction state", this->get_component_source());
+27 -4
View File
@@ -171,6 +171,27 @@ class Component {
*/
void enable_loop();
/** Thread and ISR-safe version of enable_loop() that can be called from any context.
*
* This method defers the actual enable via enable_pending_loops_ to the main loop,
* making it safe to call from ISR handlers, timer callbacks, other threads,
* or any interrupt context.
*
* @note The actual loop enabling will happen on the next main loop iteration.
* @note Only one pending enable request is tracked per component.
* @note There is no disable_loop_soon_any_context() on purpose - it would race
* against enable calls and synchronization would get too complex
* to provide a safe version that would work for each component.
*
* Use disable_loop() from the main thread only.
*
* If you need to disable the loop from ISR, carefully implement
* it in the component itself, with an ISR safe approach, and call
* disable_loop() in its next ::loop() iteration. Implementations
* will need to carefully consider all possible race conditions.
*/
void enable_loop_soon_any_context();
bool is_failed() const;
bool is_ready() const;
@@ -331,16 +352,18 @@ class Component {
/// Cancel a defer callback using the specified name, name must not be empty.
bool cancel_defer(const std::string &name); // NOLINT
// Ordered for optimal packing on 32-bit systems
float setup_priority_override_{NAN};
const char *component_source_{nullptr};
const char *error_message_{nullptr};
uint16_t warn_if_blocking_over_{WARN_IF_BLOCKING_OVER_MS}; ///< Warn if blocked for this many ms (max 65.5s)
/// State of this component - each bit has a purpose:
/// Bits 0-1: Component state (0x00=CONSTRUCTION, 0x01=SETUP, 0x02=LOOP, 0x03=FAILED)
/// Bit 2: STATUS_LED_WARNING
/// Bit 3: STATUS_LED_ERROR
/// Bits 4-7: Unused - reserved for future expansion (50% of the bits are free)
uint8_t component_state_{0x00};
float setup_priority_override_{NAN};
const char *component_source_{nullptr};
uint16_t warn_if_blocking_over_{WARN_IF_BLOCKING_OVER_MS}; ///< Warn if blocked for this many ms (max 65.5s)
const char *error_message_{nullptr};
volatile bool pending_enable_loop_{false}; ///< ISR-safe flag for enable_loop_soon_any_context
};
/** This class simplifies creating components that periodically check a state.