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

Performance Monitor causes crash on iOS 13.4 #28414

Closed
IjzerenHein opened this issue Mar 27, 2020 · 4 comments
Closed

Performance Monitor causes crash on iOS 13.4 #28414

IjzerenHein opened this issue Mar 27, 2020 · 4 comments
Labels
Needs: Triage 🔍 Resolution: Locked This issue was locked by the bot.

Comments

@IjzerenHein
Copy link
Contributor

Description

Opening the Performance Monitor causes a crash as of iOS 13.4

React Native version:

System:
    OS: macOS 10.15.3
    CPU: (8) x64 Intel(R) Core(TM) i7-8559U CPU @ 2.70GHz
    Memory: 148.41 MB / 16.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 10.16.0 - ~/.nvm/versions/node/v10.16.0/bin/node
    Yarn: 1.21.1 - /usr/local/bin/yarn
    npm: 6.9.0 - ~/.nvm/versions/node/v10.16.0/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  SDKs:
    iOS SDK:
      Platforms: iOS 13.4, DriverKit 19.0, macOS 10.15, tvOS 13.4, watchOS 6.2
    Android SDK:
      API Levels: 22, 23, 25, 26, 27, 28, 29
      Build Tools: 25.0.3, 26.0.3, 27.0.3, 28.0.2, 28.0.3, 29.0.2
      System Images: android-22 | Intel x86 Atom_64, android-23 | Google APIs Intel x86 Atom, android-25 | Android TV Intel x86 Atom, android-26 | Google APIs Intel x86 Atom_64, android-27 | Google Play Intel x86 Atom, android-28 | Google Play Intel x86 Atom, android-29 | Google APIs Intel x86 Atom, android-29 | Google Play Intel x86 Atom
      Android NDK: 21.0.6113669
  IDEs:
    Android Studio: 3.6 AI-192.7142.36.36.6241897
    Xcode: 11.4/11E146 - /usr/bin/xcodebuild
  Languages:
    Python: 2.7.15 - /usr/local/bin/python
  npmPackages:
    @react-native-community/cli: Not Found
    react: 16.11.0 => 16.11.0
    react-native: 0.62.0 => 0.62.0
  npmGlobalPackages:
    *react-native*: Not Found

Steps To Reproduce

  • Start react-native on an iOS 13.4 simulator
npx react-native init PerfCrash
cd PerfCrash
yarn ios
  • Open debug-menu
  • Tap on "Show Perf Monitor"

Expected Results

Shows Performance Monitor; and doesn't crash.

Additional info

I've debugged the issue and seem to have found the problem. JavaScriptCore has been updated in iOS 13.4 and it seems it is no longer possible to set the JavaScript Core options after initializing the VM.

The RCTJSCSetOption function now causes an EXC_BAD_ACCESS exception. The setOption function is resolved correctly but when calling it, it throws the exception. I've debugged the assembly and providing invalid values (logGC2=1 and logGC=3) do not produce a crash. So it doesn't appear to be a parsing issue, and the crash seems to occur when actually updating the option to 1.

static BOOL RCTJSCSetOption(const char *option)
{
  static RCTJSCSetOptionType setOption;
  static dispatch_once_t onceToken;

  dispatch_once(&onceToken, ^{
    /**
     * JSC private C++ static method to toggle options at runtime
     *
     * JSC::Options::setOptions - JavaScriptCore/runtime/Options.h
     */
    setOption = dlsym(RTLD_DEFAULT, "_ZN3JSC7Options9setOptionEPKc");

    if (RCT_DEBUG && setOption == NULL) {
      RCTLogWarn(@"The symbol used to enable JSC runtime options is not available in this iOS version");
    }
  });

  if (setOption) {
    return setOption(option); // <-- This throws EXC_BAD_ACCESS exception
  } else {
    return NO;
  }
}

After inspection on the JavaScriptCore code it was found that the JavaScriptCore options can no longer be changed after the VM was initialized.
https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/runtime/OptionsList.h#L74

https://raw.githubusercontent.com/WebKit/webkit/master/Source/JavaScriptCore/ChangeLog

2019-09-12  Mark Lam  <mark.lam@apple.com>

        Harden JSC against the abuse of runtime options.
        https://bugs.webkit.org/show_bug.cgi?id=201597
        <rdar://problem/55167068>

        Reviewed by Filip Pizlo.

        Linux parts contributed by Carlos Garcia Campos <cgarcia@igalia.com>.

        1. Introduce a JSC::Config struct that will be protected as ReadOnly once the
           first VM instance is constructed.  The end of the VM constructor calls
           Config::permanentlyFreeze() which will make the Config ReadOnly.
@IjzerenHein
Copy link
Contributor Author

Additionally I noticed that the GC metric in the Performance Monitor also doesn't update in iOS 13.1. So it might be broken in earlier versions as well.

tsapeta added a commit to expo/react-native that referenced this issue Mar 31, 2020
## Motivation

This PR fixes a crash when opening the Performance Monitor on iOS 13.4.
More info: facebook#28414

## How

This PR prevents the JavaScriptCore option from being set altogether.
This ensures that the performance monitor keeps working, but on iOS 13.4 and higher, it will no longer crash or show the GC usage.

## Test Plan

Tested on iOS 13.4 (simulator):

Tested on iOS 13.1 (simulator)
- Verified that the `setOption` was called, but the Performance Monitor didn't show any GC usage regardless.

## Release Notes

`[ IOS      ] [ BUGFIX      ] [RCTPerfMonitor.m] - Fix crash when enabling Performance Monitor on iOS 13.4`
@gorhom
Copy link
Contributor

gorhom commented Apr 3, 2020

@IjzerenHein good job, i have tried your solutions and works fine, would you mind submit a pr ?

@IjzerenHein
Copy link
Contributor Author

@IjzerenHein good job, i have tried your solutions and works fine, would you mind submit a pr ?

@gorhom Done! 👊

alloy pushed a commit that referenced this issue Apr 8, 2020
Summary:
This PR fixes a crash when opening the Performance Monitor on iOS 13.4.
Detailed info: #28414

## Changelog

`[iOS] [Fixed] - Fix crash when enabling Performance Monitor on iOS 13.4`

## How

This PR prevents the JavaScriptCore option from being set altogether.
This ensures that the performance monitor keeps working, but on iOS 13.4 and higher, it will no longer crash trying to show the GC usage.
Pull Request resolved: #28512

Test Plan:
Tested on iOS 13.4 (simulator):

![image](https://user-images.githubusercontent.com/6184593/77903803-c6370c00-7283-11ea-8b71-b6b6546c82f6.png)

Tested on iOS 13.1 (simulator)

![image](https://user-images.githubusercontent.com/6184593/77903499-41e48900-7283-11ea-9d14-83f67a3b7b77.png)

- Verified that the `setOption` was called, but the Performance Monitor didn't show any GC usage regardless.
- Identical PR expo#21 has been shipped and tested in Expo Client 37

Fixes #28414

Reviewed By: PeteTheHeat

Differential Revision: D20851131

Pulled By: TheSavior

fbshipit-source-id: ff96301036e8487db59f95947bbe6841fe230e1e
sasurau4 pushed a commit to sasurau4/react-native that referenced this issue Apr 15, 2020
Summary:
This PR fixes a crash when opening the Performance Monitor on iOS 13.4.
Detailed info: facebook#28414

## Changelog

`[iOS] [Fixed] - Fix crash when enabling Performance Monitor on iOS 13.4`

## How

This PR prevents the JavaScriptCore option from being set altogether.
This ensures that the performance monitor keeps working, but on iOS 13.4 and higher, it will no longer crash trying to show the GC usage.
Pull Request resolved: facebook#28512

Test Plan:
Tested on iOS 13.4 (simulator):

![image](https://user-images.githubusercontent.com/6184593/77903803-c6370c00-7283-11ea-8b71-b6b6546c82f6.png)

Tested on iOS 13.1 (simulator)

![image](https://user-images.githubusercontent.com/6184593/77903499-41e48900-7283-11ea-9d14-83f67a3b7b77.png)

- Verified that the `setOption` was called, but the Performance Monitor didn't show any GC usage regardless.
- Identical PR expo#21 has been shipped and tested in Expo Client 37

Fixes facebook#28414

Reviewed By: PeteTheHeat

Differential Revision: D20851131

Pulled By: TheSavior

fbshipit-source-id: ff96301036e8487db59f95947bbe6841fe230e1e
@arnoldc
Copy link

arnoldc commented Jun 29, 2020

This is crashing at React Native 0.60.4 , is this only fixed at latest version of react native?

@facebook facebook locked as resolved and limited conversation to collaborators Oct 1, 2021
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Oct 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Needs: Triage 🔍 Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants