From 8db810ca2e0d5286bf89aef673b74ff5e7980d23 Mon Sep 17 00:00:00 2001 From: zhongwuzw Date: Thu, 6 Jun 2019 12:07:14 +0800 Subject: [PATCH 1/3] [iOS] Removed locks && ensure everything runs on JS Thread --- React/Modules/RCTTiming.m | 80 +++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 45 deletions(-) diff --git a/React/Modules/RCTTiming.m b/React/Modules/RCTTiming.m index bfc31c63f78dfc..24b3f078bf2d46 100644 --- a/React/Modules/RCTTiming.m +++ b/React/Modules/RCTTiming.m @@ -171,6 +171,7 @@ - (dispatch_queue_t)methodQueue return RCTJSThread; } +// Called from JS Thread. - (void)invalidate { [self stopTimers]; @@ -179,20 +180,24 @@ - (void)invalidate - (void)appDidMoveToBackground { - // Deactivate the CADisplayLink while in the background. - [self stopTimers]; - _inBackground = YES; - - // Issue one final timer callback, which will schedule a - // background NSTimer, if needed. - [self didUpdateFrame:nil]; + [_bridge dispatchBlock:^{ + // Deactivate the CADisplayLink while in the background. + [self stopTimers]; + self->_inBackground = YES; + + // Issue one final timer callback, which will schedule a + // background NSTimer, if needed. + [self didUpdateFrame:nil]; + } queue:RCTJSThread]; } - (void)appDidMoveToForeground { - [self markEndOfBackgroundTaskIfNeeded]; - _inBackground = NO; - [self startTimers]; + [_bridge dispatchBlock:^{ + [self markEndOfBackgroundTaskIfNeeded]; + self->_inBackground = NO; + [self startTimers]; + } queue:RCTJSThread]; } - (void)stopTimers @@ -225,9 +230,7 @@ - (void)startTimers - (BOOL)hasPendingTimers { - @synchronized (_timers) { - return _sendIdleEvents || _timers.count > 0; - } + return _sendIdleEvents || _timers.count > 0; } - (void)didUpdateFrame:(RCTFrameUpdate *)update @@ -235,13 +238,11 @@ - (void)didUpdateFrame:(RCTFrameUpdate *)update NSDate *nextScheduledTarget = [NSDate distantFuture]; NSMutableArray<_RCTTimer *> *timersToCall = [NSMutableArray new]; NSDate *now = [NSDate date]; // compare all the timers to the same base time - @synchronized (_timers) { - for (_RCTTimer *timer in _timers.allValues) { - if ([timer shouldFire:now]) { - [timersToCall addObject:timer]; - } else { - nextScheduledTarget = [nextScheduledTarget earlierDate:timer.target]; - } + for (_RCTTimer *timer in _timers.allValues) { + if ([timer shouldFire:now]) { + [timersToCall addObject:timer]; + } else { + nextScheduledTarget = [nextScheduledTarget earlierDate:timer.target]; } } @@ -261,9 +262,7 @@ - (void)didUpdateFrame:(RCTFrameUpdate *)update [timer reschedule]; nextScheduledTarget = [nextScheduledTarget earlierDate:timer.target]; } else { - @synchronized (_timers) { - [_timers removeObjectForKey:timer.callbackID]; - } + [_timers removeObjectForKey:timer.callbackID]; } } @@ -282,10 +281,7 @@ - (void)didUpdateFrame:(RCTFrameUpdate *)update // Switch to a paused state only if we didn't call any timer this frame, so if // in response to this timer another timer is scheduled, we don't pause and unpause // the displaylink frivolously. - NSUInteger timerCount; - @synchronized (_timers) { - timerCount = _timers.count; - } + NSUInteger timerCount = _timers.count; if (_inBackground) { if (timerCount) { [self markStartOfBackgroundTaskIfNeeded]; @@ -307,18 +303,16 @@ - (void)didUpdateFrame:(RCTFrameUpdate *)update - (void)scheduleSleepTimer:(NSDate *)sleepTarget { - @synchronized (self) { - if (!_sleepTimer || !_sleepTimer.valid) { - _sleepTimer = [[NSTimer alloc] initWithFireDate:sleepTarget - interval:0 - target:[_RCTTimingProxy proxyWithTarget:self] - selector:@selector(timerDidFire) - userInfo:nil - repeats:NO]; - [[NSRunLoop currentRunLoop] addTimer:_sleepTimer forMode:NSDefaultRunLoopMode]; - } else { - _sleepTimer.fireDate = [_sleepTimer.fireDate earlierDate:sleepTarget]; - } + if (!_sleepTimer || !_sleepTimer.valid) { + _sleepTimer = [[NSTimer alloc] initWithFireDate:sleepTarget + interval:0 + target:[_RCTTimingProxy proxyWithTarget:self] + selector:@selector(timerDidFire) + userInfo:nil + repeats:NO]; + [[NSRunLoop currentRunLoop] addTimer:_sleepTimer forMode:NSDefaultRunLoopMode]; + } else { + _sleepTimer.fireDate = [_sleepTimer.fireDate earlierDate:sleepTarget]; } } @@ -362,9 +356,7 @@ - (void)timerDidFire interval:jsDuration targetTime:targetTime repeats:repeats]; - @synchronized (_timers) { - _timers[callbackID] = timer; - } + _timers[callbackID] = timer; if (_inBackground) { [self markStartOfBackgroundTaskIfNeeded]; @@ -380,9 +372,7 @@ - (void)timerDidFire RCT_EXPORT_METHOD(deleteTimer:(nonnull NSNumber *)timerID) { - @synchronized (_timers) { - [_timers removeObjectForKey:timerID]; - } + [_timers removeObjectForKey:timerID]; if (![self hasPendingTimers]) { [self stopTimers]; } From 5403a6ed05886c2e6a1c3e8d32a6bbeadc981c5c Mon Sep 17 00:00:00 2001 From: zhongwuzw Date: Thu, 6 Jun 2019 12:17:48 +0800 Subject: [PATCH 2/3] Add comments --- React/Modules/RCTTiming.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/React/Modules/RCTTiming.m b/React/Modules/RCTTiming.m index 24b3f078bf2d46..75b8f12db55c9e 100644 --- a/React/Modules/RCTTiming.m +++ b/React/Modules/RCTTiming.m @@ -178,6 +178,7 @@ - (void)invalidate _bridge = nil; } +// Called from main thread - (void)appDidMoveToBackground { [_bridge dispatchBlock:^{ @@ -191,6 +192,7 @@ - (void)appDidMoveToBackground } queue:RCTJSThread]; } +// Called from main thread - (void)appDidMoveToForeground { [_bridge dispatchBlock:^{ From 277f708530a342a031a0b0bff6ddc17e7ebfc445 Mon Sep 17 00:00:00 2001 From: zhongwuzw Date: Fri, 7 Jun 2019 08:17:19 +0800 Subject: [PATCH 3/3] Remove comments --- React/Modules/RCTTiming.m | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/React/Modules/RCTTiming.m b/React/Modules/RCTTiming.m index 75b8f12db55c9e..5e0e335f66ef4f 100644 --- a/React/Modules/RCTTiming.m +++ b/React/Modules/RCTTiming.m @@ -171,14 +171,14 @@ - (dispatch_queue_t)methodQueue return RCTJSThread; } -// Called from JS Thread. - (void)invalidate { - [self stopTimers]; - _bridge = nil; + [_bridge dispatchBlock:^{ + [self stopTimers]; + self->_bridge = nil; + } queue:RCTJSThread]; } -// Called from main thread - (void)appDidMoveToBackground { [_bridge dispatchBlock:^{ @@ -192,7 +192,6 @@ - (void)appDidMoveToBackground } queue:RCTJSThread]; } -// Called from main thread - (void)appDidMoveToForeground { [_bridge dispatchBlock:^{