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

[PLAT-8019] Prevent reporting of hangs during background launches #1307

Merged
merged 1 commit into from Feb 28, 2022

Conversation

nickdowell
Copy link
Contributor

@nickdowell nickdowell commented Feb 25, 2022

Goal

Prevent reporting of hangs during background launches.

Design

Problem: -[UIApplication applicationState] always returns UIApplicationStateBackground during the launch of UIScene based apps, which means if we rely only on this to detect foreground app launches then we would not report fatal app hangs in didFinishLaunching.

The workaround in #1263 went too far because a hang detected during prewarming or other background launch could be reported as a fatal if iOS subsequently terminated the (backgrounded) apps without warning to reclaim resources (which is quite likely.)

iOS sets a process's TASK_CATEGORY_POLICY to different values depending on the type of launch, in order to control its priority for resources such as CPU and disk. Querying this using the task_policy_get() API lets us query the category and infer the launch type.

TASK_CATEGORY_POLICY definition
/*
 * TASK_CATEGORY_POLICY:
 *
 * This provides information to the kernel about the role
 * of the task in the system.
 *
 * Parameters:
 *
 * role: Enumerated as follows:
 *
 * TASK_UNSPECIFIED is the default, since the role is not
 * inherited from the parent.
 *
 * TASK_FOREGROUND_APPLICATION should be assigned when the
 * task is a normal UI application in the foreground from
 * the HI point of view.
 * **N.B. There may be more than one of these at a given time.
 *
 * TASK_BACKGROUND_APPLICATION should be assigned when the
 * task is a normal UI application in the background from
 * the HI point of view.
 *
 * TASK_CONTROL_APPLICATION should be assigned to the unique
 * UI application which implements the pop-up application dialog.
 * There can only be one task at a time with this designation,
 * which is assigned FCFS.
 *
 * TASK_GRAPHICS_SERVER should be assigned to the graphics
 * management (window) server.  There can only be one task at
 * a time with this designation, which is assigned FCFS.
 */

Testing on various iOS versions has confirmed that TASK_FOREGROUND_APPLICATION is a reliable indicator that the process is a foreground app.

Note that this is not a suitable replacement for querying UIApplicationState at arbitraty times because the system does not update a task's policy immediately upon state transitions.

Changeset

Moves all application foreground state logic into BSG_KSCrashState and uses the task's category policy to detect foreground state at initialisation on iOS.

Removes the "runloop not yet ticked" workaround from BSGAppHangDetector, since that was allowing too many background hangs to be reported as fatal.

Removes isActive from BugsnagSystemState since the removal of [BSG_KSSystemInfo currentAppState] makes it awkward to capture this data, and it is not actually necessary for the OOM logic - if isActive is true, so is isInForeground.

BugsnagSystemState is now initialised later, since it requires BSG_KSCrashState to be ready.

Testing

Manually tested launching in foreground, for a background fetch, and prewarming (by rebooting the device.)

All app hang E2E scenarios have passed on CI - https://buildkite.com/bugsnag/bugsnag-cocoa/builds/5050

@github-actions
Copy link

Bugsnag.framework binary size decreased by 1,664 bytes from 904,072 to 902,408 🎉

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  [ = ]       0   +12% +1.62Ki    [__LINKEDIT]
  +2.6%    +676  +2.6%    +676    [__TEXT]
  +1.3%    +204  +4.2%    +204    [__DATA]
  +0.2%     +39  +0.2%     +39    __TEXT,__cstring
  +0.2%     +32  +0.2%     +32    __DATA,__cfstring
  +0.6%     +24  +0.6%     +24    Lazy Binding Info
  +0.6%     +12  +0.6%     +12    __TEXT,__stub_helper
  +0.6%     +12  +0.6%     +12    __TEXT,__stubs
  +0.5%      +8  +0.5%      +8    Indirect Symbol Table
  -0.1%      -4  -0.1%      -4    [3 Others]
  -0.4%      -8  -0.4%      -8    Function Start Addresses
  -0.3%     -11  -0.3%     -11    __TEXT,__objc_methtype
  -0.5%     -20  -0.5%     -20    __TEXT,__gcc_except_tab
  -0.5%     -20  -0.5%     -20    __TEXT,__unwind_info
  -0.4%     -32  -0.4%     -32    __DATA,__objc_selrefs
  -1.6%     -64  -1.6%     -64    __DATA,__const
  -0.4%    -112  -0.4%    -112    __TEXT,__objc_methname
  -0.3%    -144  -0.3%    -144    __DATA,__objc_const
  -0.2%    -576  -0.2%    -576    __TEXT,__text
  -0.5%    -800  -0.5%    -800    Symbol Table
  -0.4%    -880  -0.4%    -880    String Table
  -0.2% -1.62Ki  [ = ]       0    TOTAL

Generated by 🚫 Danger

@nickdowell nickdowell merged commit 4be1dc6 into next Feb 28, 2022
@nickdowell nickdowell deleted the nickdowell/background-state-detection branch February 28, 2022 09:43
This was referenced Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants