Skip to content

Commit 212d6ab

Browse files
[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>
1 parent 62255da commit 212d6ab

File tree

1 file changed

+22
-11
lines changed

1 file changed

+22
-11
lines changed

src/darwin/Framework/CHIP/MTRDevice.mm

+22-11
Original file line numberDiff line numberDiff line change
@@ -1193,23 +1193,21 @@ - (void)_handleSubscriptionEstablished
11931193
{
11941194
os_unfair_lock_lock(&self->_lock);
11951195

1196-
// We have completed the subscription work - remove from the subscription pool.
1197-
[self _clearSubscriptionPoolWork];
1198-
1199-
// reset subscription attempt wait time when subscription succeeds
1200-
_lastSubscriptionAttemptWait = 0;
1201-
if (HadSubscriptionEstablishedOnce(_internalDeviceState)) {
1202-
[self _changeInternalState:MTRInternalDeviceStateLaterSubscriptionEstablished];
1203-
} else {
1204-
MATTER_LOG_METRIC_END(kMetricMTRDeviceInitialSubscriptionSetup, CHIP_NO_ERROR);
1205-
[self _changeInternalState:MTRInternalDeviceStateInitialSubscriptionEstablished];
1196+
// If subscription had reset since this handler was scheduled, do not execute "established" logic below
1197+
if (!HaveSubscriptionEstablishedRightNow(_internalDeviceState)) {
1198+
MTR_LOG("%@ _handleSubscriptionEstablished run with internal state %lu - skipping subscription establishment logic", self, static_cast<unsigned long>(_internalDeviceState));
1199+
return;
12061200
}
12071201

1208-
[self _changeState:MTRDeviceStateReachable];
1202+
// We have completed the subscription work - remove from the subscription pool.
1203+
[self _clearSubscriptionPoolWork];
12091204

12101205
// No need to monitor connectivity after subscription establishment
12111206
[self _stopConnectivityMonitoring];
12121207

1208+
// reset subscription attempt wait time when subscription succeeds
1209+
_lastSubscriptionAttemptWait = 0;
1210+
12131211
auto initialSubscribeStart = _initialSubscribeStart;
12141212
// We no longer need to track subscribe latency for this device.
12151213
_initialSubscribeStart = nil;
@@ -2476,6 +2474,19 @@ - (void)_setupSubscriptionWithReason:(NSString *)reason
24762474
},
24772475
^(void) {
24782476
MTR_LOG("%@ got subscription established", self);
2477+
std::lock_guard lock(self->_lock);
2478+
2479+
// First synchronously change state
2480+
if (HadSubscriptionEstablishedOnce(self->_internalDeviceState)) {
2481+
[self _changeInternalState:MTRInternalDeviceStateLaterSubscriptionEstablished];
2482+
} else {
2483+
MATTER_LOG_METRIC_END(kMetricMTRDeviceInitialSubscriptionSetup, CHIP_NO_ERROR);
2484+
[self _changeInternalState:MTRInternalDeviceStateInitialSubscriptionEstablished];
2485+
}
2486+
2487+
[self _changeState:MTRDeviceStateReachable];
2488+
2489+
// Then async work that shouldn't be performed on the matter queue
24792490
dispatch_async(self.queue, ^{
24802491
// OnSubscriptionEstablished
24812492
[self _handleSubscriptionEstablished];

0 commit comments

Comments
 (0)