From 403ba262c683dab7266d54ed56807ad6074d8718 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 18 Mar 2026 21:23:47 -0400 Subject: [PATCH 1/2] [openthread] Guard InstanceLock against uninitialized semaphore (#14940) Co-authored-by: Claude Opus 4.6 (1M context) --- esphome/components/openthread/openthread.h | 4 ++++ .../components/openthread/openthread_esp.cpp | 22 ++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/esphome/components/openthread/openthread.h b/esphome/components/openthread/openthread.h index 75d8fe11fd7..bd10774fcf5 100644 --- a/esphome/components/openthread/openthread.h +++ b/esphome/components/openthread/openthread.h @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -28,6 +29,8 @@ class OpenThreadComponent : public Component { float get_setup_priority() const override { return setup_priority::WIFI; } bool is_connected() const { return this->connected_; } + /// Returns true once esp_openthread_init() has completed and the OT lock is usable. + bool is_lock_initialized() const { return this->lock_initialized_; } network::IPAddresses get_ip_addresses(); std::optional get_omr_address(); void ot_main(); @@ -51,6 +54,7 @@ class OpenThreadComponent : public Component { uint32_t poll_period_{0}; #endif std::optional output_power_{}; + std::atomic lock_initialized_{false}; bool teardown_started_{false}; bool teardown_complete_{false}; bool connected_{false}; diff --git a/esphome/components/openthread/openthread_esp.cpp b/esphome/components/openthread/openthread_esp.cpp index 9cc9223b523..27712bd86ad 100644 --- a/esphome/components/openthread/openthread_esp.cpp +++ b/esphome/components/openthread/openthread_esp.cpp @@ -8,6 +8,7 @@ #include "esp_openthread_lock.h" #include "esp_task_wdt.h" +#include "esphome/core/hal.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" @@ -81,6 +82,9 @@ void OpenThreadComponent::ot_main() { // Initialize the OpenThread stack // otLoggingSetLevel(OT_LOG_LEVEL_DEBG); ESP_ERROR_CHECK(esp_openthread_init(&config)); + // Mark lock as initialized so InstanceLock callers know it's safe to acquire. + // Must be set after esp_openthread_init() which creates the internal semaphore. + this->lock_initialized_ = true; // Fetch OT instance once to avoid repeated call into OT stack otInstance *instance = esp_openthread_get_instance(); @@ -180,7 +184,8 @@ void OpenThreadComponent::ot_main() { esp_openthread_launch_mainloop(); - // Clean up + // Clean up - reset lock flag before deinit destroys the semaphore + this->lock_initialized_ = false; esp_openthread_deinit(); esp_openthread_netif_glue_deinit(); esp_netif_destroy(openthread_netif); @@ -210,6 +215,9 @@ network::IPAddresses OpenThreadComponent::get_ip_addresses() { otInstance *OpenThreadComponent::get_openthread_instance_() { return esp_openthread_get_instance(); } std::optional InstanceLock::try_acquire(int delay) { + if (!global_openthread_component->is_lock_initialized()) { + return {}; + } if (esp_openthread_lock_acquire(delay)) { return InstanceLock(); } @@ -217,6 +225,18 @@ std::optional InstanceLock::try_acquire(int delay) { } InstanceLock InstanceLock::acquire() { + // Wait for the lock to be created by ot_main() before attempting to acquire it. + // esp_openthread_lock_acquire() will assert-crash if called before esp_openthread_init(). + constexpr uint32_t lock_init_timeout_ms = 10000; + uint32_t start = millis(); + while (!global_openthread_component->is_lock_initialized()) { + if (millis() - start > lock_init_timeout_ms) { + ESP_LOGE(TAG, "OpenThread lock not initialized after %" PRIu32 "ms, aborting", lock_init_timeout_ms); + abort(); + } + delay(10); + esp_task_wdt_reset(); + } while (!esp_openthread_lock_acquire(100)) { esp_task_wdt_reset(); } From 0594e22e3a8b144ed0c2d673369878c1ef8819e1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 18 Mar 2026 16:08:24 -1000 Subject: [PATCH 2/2] [preferences] Compile out loop() when flash_write_interval is non-zero The write_interval == 0 case (sync every loop) is only used on host or for testing. For the vast majority of real devices using the default 60s interval, loop() and the runtime branch are unnecessary overhead. Use a compile-time define to separate the two modes so that: - Normal case: no loop() override, no write_interval_ member, just set_interval - Zero interval case: loop() syncs every iteration, no interval needed --- esphome/components/preferences/__init__.py | 6 +++++- esphome/components/preferences/syncer.h | 17 +++++++---------- esphome/core/defines.h | 1 + 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/esphome/components/preferences/__init__.py b/esphome/components/preferences/__init__.py index c6bede891ab..b5af7c05500 100644 --- a/esphome/components/preferences/__init__.py +++ b/esphome/components/preferences/__init__.py @@ -21,5 +21,9 @@ CONFIG_SCHEMA = cv.Schema( @coroutine_with_priority(CoroPriority.PREFERENCES) async def to_code(config): var = cg.new_Pvariable(config[CONF_ID]) - cg.add(var.set_write_interval(config[CONF_FLASH_WRITE_INTERVAL])) + write_interval = config[CONF_FLASH_WRITE_INTERVAL] + if write_interval == 0: + cg.add_define("USE_PREFERENCES_SYNC_EVERY_LOOP") + else: + cg.add(var.set_write_interval(write_interval)) await cg.register_component(var, config) diff --git a/esphome/components/preferences/syncer.h b/esphome/components/preferences/syncer.h index 96716d3f304..e28cc8c8d54 100644 --- a/esphome/components/preferences/syncer.h +++ b/esphome/components/preferences/syncer.h @@ -8,24 +8,21 @@ namespace preferences { class IntervalSyncer final : public Component { public: +#ifdef USE_PREFERENCES_SYNC_EVERY_LOOP + void loop() override { global_preferences->sync(); } +#else void set_write_interval(uint32_t write_interval) { this->write_interval_ = write_interval; } void setup() override { - if (this->write_interval_ != 0) { - set_interval(this->write_interval_, []() { global_preferences->sync(); }); - // When using interval-based syncing, we don't need the loop - this->disable_loop(); - } - } - void loop() override { - if (this->write_interval_ == 0) { - global_preferences->sync(); - } + this->set_interval(this->write_interval_, []() { global_preferences->sync(); }); } +#endif void on_shutdown() override { global_preferences->sync(); } float get_setup_priority() const override { return setup_priority::BUS; } +#ifndef USE_PREFERENCES_SYNC_EVERY_LOOP protected: uint32_t write_interval_{60000}; +#endif }; } // namespace preferences diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 09269ea1b4d..01f33f936f1 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -117,6 +117,7 @@ #define USE_NUMBER #define USE_OUTPUT #define USE_POWER_SUPPLY +#define USE_PREFERENCES_SYNC_EVERY_LOOP #define USE_QR_CODE #define USE_SAFE_MODE_CALLBACK #define USE_SELECT