Skip to content

Commit acf4363

Browse files
Cancel the periodic Time Sync timer in Matter.framework when the controller is suspended.
We shouldn't be trying to talk to the device in that state.
1 parent 79da246 commit acf4363

File tree

1 file changed

+29
-11
lines changed

1 file changed

+29
-11
lines changed

src/darwin/Framework/CHIP/MTRDevice_Concrete.mm

+29-11
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ typedef NS_ENUM(NSUInteger, MTRDeviceWorkItemDuplicateTypeID) {
277277
#define MTRDEVICE_SUBSCRIPTION_LATENCY_NEW_VALUE_WEIGHT (1.0 / 3.0)
278278

279279
@interface MTRDevice_Concrete ()
280-
// protects against concurrent time updates by guarding timeUpdateScheduled flag which manages time updates scheduling,
280+
// protects against concurrent time updates by guarding the timeUpdateTimer field, which manages time update scheduling,
281281
// and protects device calls to setUTCTime and setDSTOffset. This can't just be replaced with "lock", because the time
282282
// update code calls public APIs like readAttributeWithEndpointID:.. (which attempt to take "lock") while holding
283283
// whatever lock protects the time sync bits.
@@ -332,7 +332,7 @@ @interface MTRDevice_Concrete ()
332332

333333
@property (nonatomic) BOOL expirationCheckScheduled;
334334

335-
@property (nonatomic) BOOL timeUpdateScheduled;
335+
@property (nonatomic, retain, readwrite, nullable) dispatch_source_t timeUpdateTimer;
336336

337337
@property (nonatomic) NSDate * estimatedStartTimeFromGeneralDiagnosticsUpTime;
338338

@@ -710,8 +710,17 @@ - (void)_setTimeOnDevice
710710

711711
- (void)_scheduleNextUpdate:(UInt64)nextUpdateInSeconds
712712
{
713+
os_unfair_lock_assert_owner(&self->_timeSyncLock);
714+
715+
auto timerSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, self.queue);
716+
717+
dispatch_source_set_timer(timerSource, dispatch_time(DISPATCH_TIME_NOW, nextUpdateInSeconds * NSEC_PER_SEC), DISPATCH_TIME_FOREVER,
718+
// Allow 3 seconds of leeway; should be plenty, in practice.
719+
static_cast<uint64_t>(3 * static_cast<double>(NSEC_PER_SEC)));
720+
713721
mtr_weakify(self);
714-
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t) (nextUpdateInSeconds * NSEC_PER_SEC)), self.queue, ^{
722+
dispatch_source_set_event_handler(timerSource, ^{
723+
dispatch_source_cancel(timerSource);
715724
mtr_strongify(self);
716725
MTR_LOG_DEBUG("%@ Timer expired, start Device Time Update", self);
717726
if (self) {
@@ -721,7 +730,7 @@ - (void)_scheduleNextUpdate:(UInt64)nextUpdateInSeconds
721730
return;
722731
}
723732
});
724-
self.timeUpdateScheduled = YES;
733+
self.timeUpdateTimer = timerSource;
725734
MTR_LOG_DEBUG("%@ Timer Scheduled for next Device Time Update, in %llu seconds", self, nextUpdateInSeconds);
726735
}
727736

@@ -731,7 +740,7 @@ - (void)_scheduleNextUpdate:(UInt64)nextUpdateInSeconds
731740
- (void)_updateDeviceTimeAndScheduleNextUpdate
732741
{
733742
os_unfair_lock_assert_owner(&self->_timeSyncLock);
734-
if (self.timeUpdateScheduled) {
743+
if (self.timeUpdateTimer != nil) {
735744
MTR_LOG_DEBUG("%@ Device Time Update already scheduled", self);
736745
return;
737746
}
@@ -749,14 +758,23 @@ - (void)_performScheduledTimeUpdate
749758
return;
750759
}
751760
// Device must not be invalidated
752-
if (!self.timeUpdateScheduled) {
761+
if (self.timeUpdateTimer == nil) {
753762
MTR_LOG_DEBUG("%@ Device Time Update is no longer scheduled, MTRDevice may have been invalidated.", self);
754763
return;
755764
}
756-
self.timeUpdateScheduled = NO;
765+
self.timeUpdateTimer = nil;
757766
[self _updateDeviceTimeAndScheduleNextUpdate];
758767
}
759768

769+
- (void)_cancelTimeUpdateTimer
770+
{
771+
std::lock_guard lock(self->_timeSyncLock);
772+
if (self.timeUpdateTimer != nil) {
773+
dispatch_source_cancel(self.timeUpdateTimer);
774+
self.timeUpdateTimer = nil;
775+
}
776+
}
777+
760778
- (NSArray<NSNumber *> *)_endpointsWithTimeSyncClusterServer
761779
{
762780
NSArray<NSNumber *> * endpointsOnDevice;
@@ -961,9 +979,7 @@ - (void)invalidate
961979

962980
[_asyncWorkQueue invalidate];
963981

964-
os_unfair_lock_lock(&self->_timeSyncLock);
965-
_timeUpdateScheduled = NO;
966-
os_unfair_lock_unlock(&self->_timeSyncLock);
982+
[self _cancelTimeUpdateTimer];
967983

968984
os_unfair_lock_lock(&self->_lock);
969985

@@ -1286,7 +1302,7 @@ - (void)_handleSubscriptionEstablished
12861302

12871303
os_unfair_lock_lock(&self->_timeSyncLock);
12881304

1289-
if (!self.timeUpdateScheduled) {
1305+
if (self.timeUpdateTimer == nil) {
12901306
[self _scheduleNextUpdate:MTR_DEVICE_TIME_UPDATE_INITIAL_WAIT_TIME_SEC];
12911307
}
12921308

@@ -4758,6 +4774,8 @@ - (void)controllerSuspended
47584774
{
47594775
[super controllerSuspended];
47604776

4777+
[self _cancelTimeUpdateTimer];
4778+
47614779
std::lock_guard lock(self->_lock);
47624780
self.suspended = YES;
47634781
[self _resetSubscriptionWithReasonString:@"Controller suspended"];

0 commit comments

Comments
 (0)