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

[ios] Fix crash when enabling Performance Monitor on iOS 13.4 #21

Merged
merged 1 commit into from
Mar 31, 2020

Conversation

IjzerenHein
Copy link

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):

image

Tested on iOS 13.1 (simulator)

image

  • 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

// As of iOS 13.4, it is no longer possible to change the JavaScriptCore
// options at runtime. The options are protected and will cause an
// exception when you try to change them after the VM has been initialized.
// https://github.com/facebook/react-native/issues/28414
Copy link
Member

Choose a reason for hiding this comment

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

So sad 😢

@tsapeta tsapeta merged commit 9ccbad5 into sdk-37 Mar 31, 2020
@tsapeta tsapeta deleted the @hein/perf-monitor-fix branch March 31, 2020 15:44
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Apr 4, 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
alloy pushed a commit to facebook/react-native that referenced this pull request 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 pull request 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
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.

3 participants