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

Workaround to open file handles for loaded assets #1287

Conversation

christophpurrer
Copy link

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

We ran into an issue that our production app was crashing on certain devices as we opened too many file descriptors (> 256). See some details here: https://wilsonmar.github.io/maximum-limits/

We found that macOS will keep image files handles open for NSImage lifetime in some cases:

  // this will keep file open only if image is loaded from app bundle
  NSString *path = [[NSBundle mainBundle] pathForResource:@"slider" ofType:@"png" inDirectory:@"assets"];
  _image = [[NSImage alloc] initWithContentsOfFile:path];

  // this will not keep file open
  _image = [[NSImage alloc] initWithContentsOfFile:@"/Users/theUser/Downloads/slider.png"];

  // this will not keep file open
  NSData *data = [NSData dataWithContentsOfFile:path];
  _image = [[NSImage alloc] initWithData:data];

This can be documented behavior, but I didn't find any reference in documentation or headers.

Changelog

[macOS] [Fixed] - Workaround to open file handles for loaded assets

Test Plan

  • build and run prod version of app
  • explore open files for Messenger process in Activity Monitor
  • confirm no assets are open

@christophpurrer christophpurrer requested a review from a team as a code owner July 25, 2022 17:08
@christophpurrer christophpurrer force-pushed the workaroundToOpenFileHandlesForLoadedAssets branch from 6699f98 to 32d862d Compare July 25, 2022 17:10
@christophpurrer christophpurrer changed the title Workaround to open file handles for loaded assets Workaround to open file handles for loaded assets [RFC] Aug 2, 2022
@christophpurrer christophpurrer changed the title Workaround to open file handles for loaded assets [RFC] [RFC] Workaround to open file handles for loaded assets Aug 10, 2022
@christophpurrer christophpurrer changed the title [RFC] Workaround to open file handles for loaded assets Workaround to open file handles for loaded assets Aug 13, 2022
@christophpurrer christophpurrer force-pushed the workaroundToOpenFileHandlesForLoadedAssets branch from 32d862d to 11e5723 Compare September 9, 2022 21:20
We ran into an issue that our production app was crashing on certain devices as we opened too many file descriptors (> 256). See some details here: https://wilsonmar.github.io/maximum-limits/

We found that macOS will keep image files handles open for `NSImage` lifetime in some cases:
```
  // this will keep file open only if image is loaded from app bundle
  NSString *path = [[NSBundle mainBundle] pathForResource:@"slider" ofType:@"png" inDirectory:@"assets"];
  _image = [[NSImage alloc] initWithContentsOfFile:path];

  // this will not keep file open
  _image = [[NSImage alloc] initWithContentsOfFile:@"/Users/theUser/Downloads/slider.png"];

  // this will not keep file open
  NSData *data = [NSData dataWithContentsOfFile:path];
  _image = [[NSImage alloc] initWithData:data];
```
This can be documented behavior, but I didn't find any reference in documentation or headers.

Test Plan:
- build and run prod version of app
- explore open files for Messenger process in Activity Monitor
- confirm no assets are open
@christophpurrer christophpurrer force-pushed the workaroundToOpenFileHandlesForLoadedAssets branch from 11e5723 to d5d8e51 Compare September 21, 2022 18:55
@Saadnajmi Saadnajmi merged commit bb8a035 into microsoft:main Sep 22, 2022
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Feb 13, 2023
We ran into an issue that our production app was crashing on certain devices as we opened too many file descriptors (> 256). See some details here: https://wilsonmar.github.io/maximum-limits/

We found that macOS will keep image files handles open for `NSImage` lifetime in some cases:
```
  // this will keep file open only if image is loaded from app bundle
  NSString *path = [[NSBundle mainBundle] pathForResource:@"slider" ofType:@"png" inDirectory:@"assets"];
  _image = [[NSImage alloc] initWithContentsOfFile:path];

  // this will not keep file open
  _image = [[NSImage alloc] initWithContentsOfFile:@"/Users/theUser/Downloads/slider.png"];

  // this will not keep file open
  NSData *data = [NSData dataWithContentsOfFile:path];
  _image = [[NSImage alloc] initWithData:data];
```
This can be documented behavior, but I didn't find any reference in documentation or headers.

Test Plan:
- build and run prod version of app
- explore open files for Messenger process in Activity Monitor
- confirm no assets are open

Co-authored-by: Scott Kyle <skyle@fb.com>
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Mar 10, 2023
We ran into an issue that our production app was crashing on certain devices as we opened too many file descriptors (> 256). See some details here: https://wilsonmar.github.io/maximum-limits/

We found that macOS will keep image files handles open for `NSImage` lifetime in some cases:
```
  // this will keep file open only if image is loaded from app bundle
  NSString *path = [[NSBundle mainBundle] pathForResource:@"slider" ofType:@"png" inDirectory:@"assets"];
  _image = [[NSImage alloc] initWithContentsOfFile:path];

  // this will not keep file open
  _image = [[NSImage alloc] initWithContentsOfFile:@"/Users/theUser/Downloads/slider.png"];

  // this will not keep file open
  NSData *data = [NSData dataWithContentsOfFile:path];
  _image = [[NSImage alloc] initWithData:data];
```
This can be documented behavior, but I didn't find any reference in documentation or headers.

Test Plan:
- build and run prod version of app
- explore open files for Messenger process in Activity Monitor
- confirm no assets are open

Co-authored-by: Scott Kyle <skyle@fb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants