Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverts "Timing: Fixes timer when app get into background (#24649)" #27073

Closed
wants to merge 1 commit into from

Conversation

radko93
Copy link
Contributor

@radko93 radko93 commented Oct 31, 2019

Summary

This PR reverts commit 3382984 that is causing #26696 #26995.

app would be closed immediately after going to background on iOS 13.1/13.2 and was investigated by @minhtc #26696 (comment). The commit that is being reverted is apparently causing the app to be closed immediately. This has to be reverted in master separately as the file differs there.

Similar PR for 0.61. branch #27065

Changelog

[iOS] [Fixed] - Fix apps crashing on iOS 13.x when running timer in the background

Test Plan

Try this snippet on iOS 13.1/13.2, the app should not crash anymore

…4649)"

Co-Authored-By: �HackerMeo <minhtc@users.noreply.github.com>
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 31, 2019
@react-native-bot react-native-bot added Bug Platform: iOS iOS applications. labels Oct 31, 2019
@cpojer
Copy link
Contributor

cpojer commented Nov 4, 2019

Duplicate of #27065

@cpojer cpojer marked this as a duplicate of #27065 Nov 4, 2019
@cpojer cpojer closed this Nov 4, 2019
@grabbou
Copy link
Contributor

grabbou commented Nov 4, 2019

Not really. The #27065 is towards a 0.61-stable branch (so we can have easier time cherry-picking). Here, it's something that should be imported if we revert from master.

@grabbou grabbou reopened this Nov 4, 2019
@cpojer
Copy link
Contributor

cpojer commented Nov 5, 2019

Thanks for the explanation, I didn't realize they were meant for different branches, my mistake.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @radko93 in e1d03b4.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Nov 5, 2019
@cristianoccazinsp
Copy link
Contributor

Sorry to bring this up, does this bug only affect RN 0.61? I'm seeing lots of strange "no background task exists with identifier ..." logs with RN 0.60 and iOS 13.x. These logs were not here before. I wonder if these background timers have anything to do with it.

@craftzdog
Copy link

craftzdog commented Aug 27, 2020

Hi, I'm from #25083.
@zhongwuzw I upgraded my RN project and found that the patch got reverted. Now, how do you let JS keep running for a few seconds in background on iOS?

Update: Maybe just adding it should work in RN0.62:

- (void)applicationDidEnterBackground:(UIApplication *)application
{
  UIBackgroundTaskIdentifier taskId = [application beginBackgroundTaskWithExpirationHandler:nil];
  dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
    // do nothing for 4.7 seconds
    [NSThread sleepForTimeInterval:4.7f];
    [application endBackgroundTask:taskId];
  });
}

@radko93 radko93 deleted the fix/timing-revert branch August 27, 2020 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants