From 212d6abf7be4e406447d6039efb22bdd2f9c619d Mon Sep 17 00:00:00 2001
From: Jeff Tung <100387939+jtung-apple@users.noreply.github.com>
Date: Tue, 6 Aug 2024 17:39:44 -0700
Subject: [PATCH] [Darwin] MTRDevice subscription estalished handler should
 change state before async (#34797)

* [Darwin] MTRDevice subscription estalished handler should change state before async

* Update src/darwin/Framework/CHIP/MTRDevice.mm

Co-authored-by: Kiel Oleson <kielo@apple.com>

* Addressed PR review - moved code comment to correct place

---------

Co-authored-by: Kiel Oleson <kielo@apple.com>
---
 src/darwin/Framework/CHIP/MTRDevice.mm | 33 +++++++++++++++++---------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm
index b8ef8694825e1f..07b695cd3c80d4 100644
--- a/src/darwin/Framework/CHIP/MTRDevice.mm
+++ b/src/darwin/Framework/CHIP/MTRDevice.mm
@@ -1193,23 +1193,21 @@ - (void)_handleSubscriptionEstablished
 {
     os_unfair_lock_lock(&self->_lock);
 
-    // We have completed the subscription work - remove from the subscription pool.
-    [self _clearSubscriptionPoolWork];
-
-    // reset subscription attempt wait time when subscription succeeds
-    _lastSubscriptionAttemptWait = 0;
-    if (HadSubscriptionEstablishedOnce(_internalDeviceState)) {
-        [self _changeInternalState:MTRInternalDeviceStateLaterSubscriptionEstablished];
-    } else {
-        MATTER_LOG_METRIC_END(kMetricMTRDeviceInitialSubscriptionSetup, CHIP_NO_ERROR);
-        [self _changeInternalState:MTRInternalDeviceStateInitialSubscriptionEstablished];
+    // If subscription had reset since this handler was scheduled, do not execute "established" logic below
+    if (!HaveSubscriptionEstablishedRightNow(_internalDeviceState)) {
+        MTR_LOG("%@ _handleSubscriptionEstablished run with internal state %lu - skipping subscription establishment logic", self, static_cast<unsigned long>(_internalDeviceState));
+        return;
     }
 
-    [self _changeState:MTRDeviceStateReachable];
+    // We have completed the subscription work - remove from the subscription pool.
+    [self _clearSubscriptionPoolWork];
 
     // No need to monitor connectivity after subscription establishment
     [self _stopConnectivityMonitoring];
 
+    // reset subscription attempt wait time when subscription succeeds
+    _lastSubscriptionAttemptWait = 0;
+
     auto initialSubscribeStart = _initialSubscribeStart;
     // We no longer need to track subscribe latency for this device.
     _initialSubscribeStart = nil;
@@ -2476,6 +2474,19 @@ - (void)_setupSubscriptionWithReason:(NSString *)reason
                                },
                                ^(void) {
                                    MTR_LOG("%@ got subscription established", self);
+                                   std::lock_guard lock(self->_lock);
+
+                                   // First synchronously change state
+                                   if (HadSubscriptionEstablishedOnce(self->_internalDeviceState)) {
+                                       [self _changeInternalState:MTRInternalDeviceStateLaterSubscriptionEstablished];
+                                   } else {
+                                       MATTER_LOG_METRIC_END(kMetricMTRDeviceInitialSubscriptionSetup, CHIP_NO_ERROR);
+                                       [self _changeInternalState:MTRInternalDeviceStateInitialSubscriptionEstablished];
+                                   }
+
+                                   [self _changeState:MTRDeviceStateReachable];
+
+                                   // Then async work that shouldn't be performed on the matter queue
                                    dispatch_async(self.queue, ^{
                                        // OnSubscriptionEstablished
                                        [self _handleSubscriptionEstablished];