Support background timers on iOS via NSTimer

[jamesreggio]

This PR is a minimalistic change to RCTTiming that causes it to switch exclusively to NSTimer (i.e., the 'sleep timer') in order to continue triggering timers when the app has moved to the background.

Motivation:

Many people have expressed a desire for background timer support on iOS. (See #1282, #167, and #16493). In our app — a podcast/audio player — we use background timers to ensure that we never lose track of the user's playback position, should the app crash or be terminated by the OS.

The RCTTimer module uses a RN-managed CADisplayLink if the next requested timer is less than a second away; otherwise, it switches to an NSTimer (which is refers to as a 'sleep timer' in source). The RN-managed CADisplayLink is always disabled when the app goes to the background (and thus cannot be used); however, the NSTimer will still issue its callbacks in the background.

This PR adds a flag to track whether the app is in the background, and if so, all timers are routed through NSTimer until the app returns to the foreground. @vishnevskiy at Discord opened a similar PR (#16493) that implements a drop-in for CADisplayLink which falls back to NSTimer, but I decided to incorporate the background-NSTimer logic directly into RCTTimer, since NSTimer is already in use.

It's worth noting that the background NSTimer may not fire as often as requested — it may give the appearance of lagging depending upon your app's priority in the background. For our audio app, NSTimer fires exactly on schedule if there's an open AVAudioSession and audio is playing; if audio is not playing, it fires about half as often as requested, which is still adequate for networking polling and other tasks.

It's worth noting that background timers only function as long as an app is actually running in the background. Apple offers a variety of Background Modes (which can be toggled in the Capabilities section of the target inspector in Xcode), and the app will need to be legitimately using one of these modes in order for this change to provide any value — otherwise it will be terminated within a couple of seconds of moving to the background.

The good thing about this change is that for apps that do perform essential computation in support of their Background Mode, they can now use setTimeout and setInterval without problem — whereas in the past, neither would ever trigger their callback until the app returns to the foreground.

Open Question:

Despite the demand for this timer behavior, it's not clear whether this should be specifically opt-in. There's nothing specifically breaking about this change, but an app that abuses timers may lead to increased battery consumption if they now fire in the background.

On the other hand, if we were to make this an opt-in feature, it's not clear how to do it other than to expose a top-level function to toggle it (and that feels hacky and undiscoverable). RN's timers are based upon standard JS functions which cannot have their parameters modified to take additional flags.

This is no longer a concern due to the fact that the app must have a valid Background Mode, which is already a multi-step opt-in process.

Test Plan:

We've been using this patch in our production app without issue since January 2018.

I've tested on a real iPhone 6 running iOS 10 and a real iPhone 8 running iOS 11. It also functions as expected on various simulators.

To demonstrate the difference, here is a before and after video.

Before	After

Release Notes:

[IOS] [ENHANCEMENT] [Timers] - Support background timers on iOS

[radex]

Despite the demand for this timer behavior, it's not clear whether this should be specifically opt-in. There's nothing specifically breaking about this change, but an app that abuses timers may lead to increased battery consumption if they now fire in the background.

Not really. Audio apps are something of an exception, as iOS allows you to continue executing code in background. Almost all apps will be suspended within a few moments of being put to background. I'd assume if you're playing in Xcode with background modes, you kind of know what you're doing.

It's still worth pointing it out in the documentation that background behavior of timers may not be strictly relied upon

[hramos]

@jamesreggio can you rebase past bc8d052 and 5068dfc? I'm interested in seeing if this will pass the TimingModuleTest in test_android, but that job is failing due to an issue we already fixed in master.

[hramos]

I'm OK with making this change, but open to hearing any concerns related to the background behavior James highlighted here.

I can merge once tests are passing.

[janicduplessis]

@jamesreggio Do you know how this compares to android? Also will the timers fire at all if the app does not declare any background modes?

[jamesreggio]

Thanks for the thoughtful replies, everybody.

Upon further inspection, you need to be actively using an approved Background Mode for this to provide any value whatsoever. (Otherwise, the app will be terminated within a couple of seconds of reaching the background, as expected.)

However, this change remains important for apps that legitimately use a Background Mode (like our audio app), because without it, any callbacks supplied to setTimeout or setInterval will fail to trigger until the app returns to the foreground. I've updated the PR description to this effect.

I agree with @radex that there's no need to make this behavior opt-in, given the necessity of opting in to a Background Mode within Xcode. This merely just causes React Native to 'do the right thing' when you're in the background.

@janicduplessis — for apps without any Background Modes, this change will cause timers to fire in the couple of seconds between moving to the background and being suspended, which is still helpful if you're doing anything with a short setTimeout in response to a shift to background. If the timer doesn't trigger before the background suspension occurs, the timer will trigger when the app is returned to the foreground, which is the same behavior that exists today.

In terms of Android, allow me to preface by saying that I'm working off of recollections of Android's timing module from several months ago. However, if I recall correctly, Android requires the use of external libraries to support long-lived or background timers — hence the common YellowBox warning to that effect. In a sense, Android's timer implementation (via Choreographer.postFrameCallback) contains only the CADisplayLink half of the iOS implementation. If Android was modified to support long-lived timers via android.os.Handler.postDelayed plus an appropriate WakeLock, vis-a-vis react-native-background-timer, I think that would be analogous to this change in iOS. However, I don't have the time to take a stab at it right now.

@hramos — I rebased against master just now (and picked up the two commits you mentioned), however tests are still failing. They seem to be spurious, since they're either related to Android (which is unchanged), or related to Metro failures (which are just flaky in the first place). Please advise if you'd like me to rebase again or make further changes.

[hramos]

@jamesreggio they do look unrelated. No need to rebase for now, since these failures are showing up on master as well.

[jamesreggio]

Sounds great, Héctor. I think this is ready to go, then?

…

On Mon, Sep 24, 2018, 5:44 PM Héctor Ramos ***@***.***> wrote: @jamesreggio <https://github.com/jamesreggio> they do look unrelated. No need to rebase for now, since these failures are showing up on master as well. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21211 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAyLve7HFTJeDIrON5dFwV0O94aTTLOaks5ueVJZgaJpZM4Ww-MP> .

[jamesreggio]

(@hramos — sorry, didn't remember your handle to @-mention you from my email reply.)

I think this is ready to go. Let me know if there is anything else you need on my end.

[jamesreggio]

Hi again — this is ready to go. @hramos, do you mind initiating Facebook import, or letting me know whether any unresolved objections remain? I'd like to field them before I lose familiarity with these systems.

[cojo]

Hi @jamesreggio and @hramos - is any update available on the status of this PR and if it can be merged for an upcoming release?

We are using React Native to develop an app which makes heavy use of dynamic background audio, and this patch is a huge quality-of-life improvement for our customers - so much so that we are manually applying this patch in our XCode / iOS buildflow as a workaround in the meantime.

Thanks for your work on this!

[jamesreggio]

It's ready — just waiting on FB.

Glad to hear it's working for you, though!

[cojo]

Hi @jamesreggio - we're in final testing for our release including this patch (applied manually), and it appears to be causing hard crashes in at least a couple of edge cases.

I'm not especially familiar with NSTimer / RCTTiming and their intricacies, but what we are seeing is that the NSRunLoop RCTTiming is scheduling the sleepTimer on is hard-crashing every so often - in particular it seems to happen most often when a system modal is displayed by iOS for permissions (e.g. microphone permissions, contact permissions, notification permissions system dialogs have all triggered it for us - but not 100% of the time).

I've included excerpts from two different crash dumps below. They both point to line 278, in scheduleSleepTimer, as the culprit:
[[NSRunLoop currentRunLoop] addTimer:_sleepTimer forMode:NSDefaultRunLoopMode];
I've also tried changing this to use NSRunLoopCommonModes but it didn't appear to help.

I know this code wasn't directly changed by your patch, but it seems closely related. Do you happen to have any thoughts / ideas for us to try? We'll continue investigating on our side as well.

Thanks!

Crash excerpt 1:

OS Version: iOS 12.0 (16A366)
Report Version: 104

Exception Type: EXC_BAD_ACCESS (SIGBUS)
Exception Codes: BUS_NOOP at 0x0000000c756b3620
Crashed Thread: 3

Application Specific Information:
Attempted to dereference garbage pointer 0xc756b3620.
Originated at or in a subcall of __cxa_throw

Thread 3 Crashed:
0   libobjc.A.dylib                 0x335c00430         objc_retain
1   CoreFoundation                  0x337761878         __CFBasicHashAddValue
2   CoreFoundation                  0x337760a70         CFBasicHashAddValue
3   CoreFoundation                  0x3376c0e98         CFRunLoopAddTimer
4   OurApp                       0x201124ba8         -[RCTTiming scheduleSleepTimer:] (RCTTiming.m:278)
5   OurApp                       0x201124d84         -[RCTTiming createTimer:duration:jsSchedulingTime:repeats:]
6   CoreFoundation                  0x33773d660         __invoking___
7   CoreFoundation                  0x337619980         -[NSInvocation invoke]
8   CoreFoundation                  0x33761a564         -[NSInvocation invokeWithTarget:]
9   OurApp                       0x201112468         -[RCTModuleMethod invokeWithBridge:module:arguments:] (RCTModuleMethod.mm:550)
10  OurApp                       0x2011592dc         facebook::react::invokeInner(RCTBridge*, RCTModuleData*, unsigned int, folly::dynamic const&) (RCTNativeModule.mm:104)
11  OurApp                       0x201159038         [inlined] operator() (RCTNativeModule.mm:71)
12  OurApp                       0x201159038         ___ZN8facebook5react15RCTNativeModule6invokeEjON5folly7dynamicEi_block_invoke (RCTNativeModule.mm:65)
13  OurApp                       0x201158f0c         facebook::react::RCTNativeModule::invoke(unsigned int, folly::dynamic&&, int) (RCTNativeModule.mm:77)
14  OurApp                       0x2011999a4         facebook::react::JsToNativeBridge::callNativeModules(facebook::react::JSExecutor&, folly::dynamic&&, bool) (NativeToJsBridge.cpp:52)
15  OurApp                       0x201194694         facebook::react::JSCExecutor::callNativeModules(facebook::react::Value&&) (JSCExecutor.cpp:557)
16  OurApp                       0x201194acc         [inlined] operator() (JSCExecutor.cpp:620)
17  OurApp                       0x201194acc         facebook::react::JSCExecutor::callFunction(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, folly::dynamic const&) (JSCExecutor.cpp:606)
18  OurApp                       0x20119adec         std::__1::function<void (facebook::react::JSExecutor*)>::operator()(facebook::react::JSExecutor*) const (functional:1913)
19  OurApp                       0x20110d8b4         facebook::react::tryAndReturnError(std::__1::function<void ()> const&) (RCTCxxUtils.mm:95)
20  OurApp                       0x201103498         facebook::react::RCTMessageThread::tryFunc(std::__1::function<void ()> const&) (RCTMessageThread.mm:60)
21  CoreFoundation                  0x3376c4408         __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__
22  CoreFoundation                  0x3376c3d08         __CFRunLoopDoBlocks
23  CoreFoundation                  0x3376bf220         __CFRunLoopRun
24  CoreFoundation                  0x3376be5b8         CFRunLoopRunSpecific
25  OurApp                       0x2010e427c         +[RCTCxxBridge runRunLoop] (RCTCxxBridge.mm:252)
26  Foundation                      0x338c773b0         __NSThread__start__
27  libsystem_pthread.dylib         0x33707b2fc         _pthread_body
28  libsystem_pthread.dylib         0x33707b25c         _pthread_start

Thread 0
0   libsystem_kernel.dylib          0x336f4fed0         mach_msg_trap
1   libsystem_kernel.dylib          0x336f4f3a8         mach_msg
2   CoreFoundation                  0x3376c3fb0         __CFRunLoopServiceMachPort
3   CoreFoundation                  0x3376bee4c         __CFRunLoopRun
4   CoreFoundation                  0x3376be5b8         CFRunLoopRunSpecific
5   GraphicsServices                0x33bc41584         GSEventRunModal
6   UIKitCore                       0x3905f4558         UIApplicationMain
7   OurApp                       0x200f39ac8         main (main.m:14)
8   libdyld.dylib                   0x336ce4b94         start

Thread 1 name: com.apple.uikit.eventfetch-thread
0   libsystem_kernel.dylib          0x336f4fed0         mach_msg_trap
1   libsystem_kernel.dylib          0x336f4f3a8         mach_msg
2   CoreFoundation                  0x3376c3fb0         __CFRunLoopServiceMachPort
3   CoreFoundation                  0x3376bee4c         __CFRunLoopRun
4   CoreFoundation                  0x3376be5b8         CFRunLoopRunSpecific
5   Foundation                      0x338b446a4         -[NSRunLoop(NSRunLoop) runMode:beforeDate:]
6   Foundation                      0x338b44550         -[NSRunLoop(NSRunLoop) runUntilDate:]
7   UIKitCore                       0x390551ac0         -[UIEventFetcher threadMain]
8   Foundation                      0x338c773b0         __NSThread__start__
9   libsystem_pthread.dylib         0x33707b2fc         _pthread_body
10  libsystem_pthread.dylib         0x33707b25c         _pthread_start

Thread 2
0   libsystem_kernel.dylib          0x336f5bb9c         __workq_kernreturn
1   libsystem_pthread.dylib         0x33707c114         _pthread_wqthread

Crash excerpt 2:

OS Version: iOS 12.0 (16A366)
Report Version: 104

Exception Type: EXC_BAD_ACCESS (SIGBUS)
Exception Codes: BUS_NOOP at 0x000000035339b7a0
Crashed Thread: 0

Application Specific Information:
Attempted to dereference garbage pointer 0x35339b7a0.
Originated at or in a subcall of __cxa_throw

Thread 0 Crashed:
0   libobjc.A.dylib                 0x335c00430         objc_retain
1   CoreFoundation                  0x337663f2c         _CFArrayReplaceValues
2   CoreFoundation                  0x337664620         CFArrayInsertValueAtIndex
3   CoreFoundation                  0x3376c1018         __CFRepositionTimerInMode
4   CoreFoundation                  0x3376c0f28         CFRunLoopAddTimer
5   OurApp                       0x200c9cba8         -[RCTTiming scheduleSleepTimer:] (RCTTiming.m:278)
6   OurApp                       0x200c9c8c4         -[RCTTiming didUpdateFrame:] (RCTTiming.m:253)
7   OurApp                       0x200c9bcf8         -[_RCTTimingProxy timerDidFire] (RCTTiming.m:89)
8   Foundation                      0x338c785fc         __NSFireTimer
9   CoreFoundation                  0x3376c4bf0         __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__
10  CoreFoundation                  0x3376c4920         __CFRunLoopDoTimer
11  CoreFoundation                  0x3376c4154         __CFRunLoopDoTimers
12  CoreFoundation                  0x3376bf030         __CFRunLoopRun
13  CoreFoundation                  0x3376be5b8         CFRunLoopRunSpecific
14  GraphicsServices                0x33bc41584         GSEventRunModal
15  UIKitCore                       0x3905f4558         UIApplicationMain
16  OurApp                       0x200ab1ac8         main (main.m:14)
17  libdyld.dylib                   0x336ce4b94         start

Thread 1 name: com.apple.uikit.eventfetch-thread
0   libsystem_kernel.dylib          0x336f4fed0         mach_msg_trap
1   libsystem_kernel.dylib          0x336f4f3a8         mach_msg
2   CoreFoundation                  0x3376c3fb0         __CFRunLoopServiceMachPort
3   CoreFoundation                  0x3376bee4c         __CFRunLoopRun
4   CoreFoundation                  0x3376be5b8         CFRunLoopRunSpecific
5   Foundation                      0x338b446a4         -[NSRunLoop(NSRunLoop) runMode:beforeDate:]
6   Foundation                      0x338b44550         -[NSRunLoop(NSRunLoop) runUntilDate:]
7   UIKitCore                       0x390551ac0         -[UIEventFetcher threadMain]
8   Foundation                      0x338c773b0         __NSThread__start__
9   libsystem_pthread.dylib         0x33707b2fc         _pthread_body
10  libsystem_pthread.dylib         0x33707b25c         _pthread_start

Thread 2 name: com.facebook.react.JavaScript
0   libsystem_kernel.dylib          0x336f5c964         kevent_id
1   libdispatch.dylib               0x336c20648         _dispatch_kq_poll
2   libdispatch.dylib               0x336c1fbd4         _dispatch_event_loop_poke$VARIANT$mp
3   OurApp                       0x200cd0ec0         facebook::react::RCTNativeModule::invoke(unsigned int, folly::dynamic&&, int) (RCTNativeModule.mm:79)
4   OurApp                       0x200d119a4         facebook::react::JsToNativeBridge::callNativeModules(facebook::react::JSExecutor&, folly::dynamic&&, bool) (NativeToJsBridge.cpp:52)
5   OurApp                       0x200d0c694         facebook::react::JSCExecutor::callNativeModules(facebook::react::Value&&) (JSCExecutor.cpp:557)
6   OurApp                       0x200d0cacc         [inlined] operator() (JSCExecutor.cpp:620)
7   OurApp                       0x200d0cacc         facebook::react::JSCExecutor::callFunction(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, folly::dynamic const&) (JSCExecutor.cpp:606)
8   OurApp                       0x200d12dec         std::__1::function<void (facebook::react::JSExecutor*)>::operator()(facebook::react::JSExecutor*) const (functional:1913)
9   OurApp                       0x200c858b4         facebook::react::tryAndReturnError(std::__1::function<void ()> const&) (RCTCxxUtils.mm:95)
10  OurApp                       0x200c7b498         facebook::react::RCTMessageThread::tryFunc(std::__1::function<void ()> const&) (RCTMessageThread.mm:60)
11  CoreFoundation                  0x3376c4408         __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__
12  CoreFoundation                  0x3376c3d08         __CFRunLoopDoBlocks
13  CoreFoundation                  0x3376bf220         __CFRunLoopRun
14  CoreFoundation                  0x3376be5b8         CFRunLoopRunSpecific
15  OurApp                       0x200c5c27c         +[RCTCxxBridge runRunLoop] (RCTCxxBridge.mm:252)
16  Foundation                      0x338c773b0         __NSThread__start__
17  libsystem_pthread.dylib         0x33707b2fc         _pthread_body
18  libsystem_pthread.dylib         0x33707b25c         _pthread_start

[cojo]

Quick update - we managed to eliminate the most common / egregious crashes by changing scheduleSleepTimer to use a lock / synchronize to be more thread-safe:

- (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];
    }
  }
}

Again, my knowledge of this system is quite limited so if anyone here has thoughts on the issue / a better long-term fix please do let me know. It seems like this might not be the only thread-safety issue that might result from this change, so I'll keep this thread posted on anything else we find once we're live to a larger scale.

[cojo]

We've found one additional crash since launch which appears to be fixed by also @synchronize'ing all _timers accesses. Let me know if it's helpful to add to your PR branch what we've put together and I'd be happy to do so.

Thanks!

[hramos]

Sorry for the delay. Let's workout whether the changes by @cojo need to be included in this PR, and then I'll import this for further review from our team.

[hramos]

Importing to put it on my queue. I can always re-import to get any additional commits.

[facebook-github-bot]

@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[cojo]

Thanks @hramos - if it's helpful, here is the "full patch" file we're applying in our build process internally here at this point to resolve all the crashes we have found so far: react-native-timers.patch - I'm not 100% sure how to integrate these changes with this existing PR but I'm happy to do so.

[cpojer]

What's the status of this PR?

[jamesreggio]

Looks like I should probably incorporate @cojo's feedback.

I had other, far simpler PRs open that weren't receiving any motion post-import from FB (until today), so I didn't prioritize making changes here just to have them sit.

Now that I see you're on it, @cpojer, I'll incorporate @cojo's patch, smoke test, and push.

[cojo]

Just to follow up, I can now confirm on our side that my latest patch (the one on Dec 11 2018) has solved all remaining crashes we're aware of on production (for our app, at least).

Thanks again @jamesreggio for getting this set up!

[cpojer]

Could you add examples on the usage of this feature to RNTester as well?

[cpojer]

@jamesreggio hey! Did you have a chance to look at my last comment about adding a manual test case for this?

[cpojer]

@jamesreggio @cojo just pinging one more time. Could either of you update this PR so we can actually merge it?

[cojo]

@cpojer If @jamesreggio is unavailable to update the PR, I can set aside some time to apply my changes directly; however, this PR appears to have been opened from a personal fork of his.

My understanding is I would have to PR my changes to his fork first before it would be applied to this PR. Is that correct? If so, if he's out of town, that might not work either. Is there another way for me to apply my changes to this PR or would it be better to open my own PR with the full changeset and reference this PR from there?

Thanks!

[hramos]

Your own PR should be fine.

[cojo]

@hramos @cpojer
Following up on this; per your request I have created PR #23674 which incorporates the crash fixes. I just noticed your request re: RNTester - I'm not particularly familiar with that part of the project / code but I could take a look if it is required before merging. Is there a particular test case you have in mind?
Since this is being built directly / "seamlessly" into RCTTiming and will just "work as expected" for background-mode users going forward, the only thing that occurs off the top of my head would be to add an incrementing timer similar to @jamesreggio 's GIF in the original PR description - is that what you had in mind?