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

Don't create a promise when preloading package settings in snapshot #19418

Merged
merged 1 commit into from May 30, 2019

Conversation

Projects
None yet
2 participants
@as-cii
Copy link
Member

commented May 29, 2019

Fixes #19374

Promise creation is forbidden within mksnapshot starting from Electron 3 (see electron/libchromiumcontent#363, nodejs/node#13242 and electron/electron#18420). Since preloading a package's settings is a synchronous action anyway, we just avoid instantiating a new Promise when calling loadSettings.

This fixes the crashes we were observing in #19373 when generating a new snapshot, and it is a prerequisite for shipping the Electron 3 upgrade after a hotfix with electron/electron#18426 is released.

Don't create a promise when preloading package settings in snapshot
Promise creation is forbidden within `mksnapshot` (see 
electron/libchromiumcontent#363, 
nodejs/node#13242 and 
electron/electron#18420). Since preloading a 
package's settings is a synchronous action anyway, we just avoid 
instantiating a new Promise when calling `loadSettings`.

@as-cii as-cii requested a review from rafeca May 29, 2019

@rafeca

rafeca approved these changes May 29, 2019

Copy link
Contributor

left a comment

😍

@as-cii

This comment has been minimized.

Copy link
Member Author

commented May 30, 2019

I think the failures are unrelated to this pull request, so I'll go ahead and merge.

@as-cii as-cii merged commit a2e71f4 into master May 30, 2019

1 of 2 checks passed

Atom Pull Requests #20190529.4 failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@as-cii as-cii deleted the as/no-promises-in-mksnapshot branch May 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.