-
Notifications
You must be signed in to change notification settings - Fork 16.6k
fix: add macos memory query fallback patch to avoid crash #47765
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nice one! Maybe this needs to be called out in the release notes more than a normal fix ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great patch description and overview. Thanks @clavin!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to fast-track this one, since it has independent approvals
|
Release Notes Persisted
|
|
I have automatically backported this PR to "38-x-y", please check out #47783 |
|
I have automatically backported this PR to "37-x-y", please check out #47784 |
Description of Change
In https://crrev.com/c/6274964 the implementation for querying the physical memory on macOS was changed to use a sysctl call. In that same change the sysctl call was added to the sanbox allowlist.
This causes a problematic behavior: if an app that's running the old implementation (no sandbox exclusion for that sysctl call) gets swapped with the new implementation (uses new sysctl call) while it's running, then new child processes will trigger a sandbox permission error when calling the new method.
While this "hot-swapping" behavior isn't supported, many enterprise update scripts may do this anyways, triggering an unfortunate user experience where child processes can never spawn but the browser process continues to live and terminate them (until the app is restarted).
This PR adds a patch to incorporate the old implementation as a fallback, giving apps a reasonable grace period as they update Electron versions.
Checklist
npm testpassesRelease Notes
Notes: Fixed a child process crash on macOS when the running application is replaced with one that has a newer implementation triggering the sandbox