[openthread] Fix InstanceLock releasing the lock twice on try_acquire (#16980)

esphome/components/openthread/openthread.cpp

@@ -227,7 +227,7 @@ bool OpenThreadComponent::teardown() {
       ESP_LOGW(TAG, "Failed to acquire OpenThread lock during teardown, leaking memory");
       return true;
     }
-    otInstance *instance = lock->get_instance();
+    otInstance *instance = lock.get_instance();
     otSrpClientClearHostAndServices(instance);
     otSrpClientBuffersFreeAllServices(instance);
     global_openthread_component = nullptr;

esphome/components/openthread/openthread.h

@@ -86,19 +86,32 @@ class OpenThreadSrpComponent : public Component {
   void *pool_alloc_(size_t size);
 };
 
+// RAII guard for the OpenThread API lock. Modeled on std::unique_lock: the
+// guard may or may not own the lock (try_acquire can fail), so check it with
+// operator bool before use. Non-copyable and non-movable: the factories return
+// by value via guaranteed copy elision, so a guard is never duplicated and the
+// lock is released exactly once, when the owning guard goes out of scope.
 class InstanceLock {
  public:
-  static std::optional<InstanceLock> try_acquire(int delay);
+  // May fail to acquire within delay ms; check the returned guard with operator bool.
+  static InstanceLock try_acquire(int delay);
+  // Blocks until the lock is held.
   static InstanceLock acquire();
+  InstanceLock(const InstanceLock &) = delete;
+  InstanceLock(InstanceLock &&) = delete;
+  InstanceLock &operator=(const InstanceLock &) = delete;
+  InstanceLock &operator=(InstanceLock &&) = delete;
   ~InstanceLock();
 
-  // Returns the global openthread instance guarded by this lock
+  explicit operator bool() const { return this->owns_; }
+
+  // Returns the global openthread instance. Only valid on an owning guard
+  // (operator bool is true); the instance must not be used without the lock held.
   otInstance *get_instance();
 
  private:
-  // Use a private constructor in order to force the handling
+  explicit InstanceLock(bool owns) : owns_(owns) {}
-  // of acquisition failure
+  bool owns_;
-  InstanceLock() {}
 };
 
 }  // namespace esphome::openthread

esphome/components/openthread/openthread_esp.cpp

@@ -216,14 +216,11 @@ network::IPAddresses OpenThreadComponent::get_ip_addresses() {
 // not thread safe, only use in read-only use cases
 otInstance *OpenThreadComponent::get_openthread_instance_() { return esp_openthread_get_instance(); }
 
-std::optional<InstanceLock> InstanceLock::try_acquire(int delay) {
+InstanceLock InstanceLock::try_acquire(int delay) {
   if (!global_openthread_component->is_lock_initialized()) {
-    return {};
+    return InstanceLock(false);
   }
-  if (esp_openthread_lock_acquire(delay)) {
+  return InstanceLock(esp_openthread_lock_acquire(delay));
-    return InstanceLock();
-  }
-  return {};
 }
 
 InstanceLock InstanceLock::acquire() {
@@ -242,12 +239,16 @@ InstanceLock InstanceLock::acquire() {
   while (!esp_openthread_lock_acquire(100)) {
     esp_task_wdt_reset();
   }
-  return InstanceLock();
+  return InstanceLock(true);
 }
 
 otInstance *InstanceLock::get_instance() { return esp_openthread_get_instance(); }
 
-InstanceLock::~InstanceLock() { esp_openthread_lock_release(); }
+InstanceLock::~InstanceLock() {
+  if (this->owns_) {
+    esp_openthread_lock_release();
+  }
+}
 
 }  // namespace esphome::openthread
 #endif

esphome/components/openthread_info/openthread_info_text_sensor.h

@@ -17,7 +17,7 @@ class OpenThreadInstancePollingComponent : public PollingComponent {
       return;
     }
 
-    this->update_instance(lock->get_instance());
+    this->update_instance(lock.get_instance());
   }
   float get_setup_priority() const override { return setup_priority::AFTER_WIFI; }