-
Notifications
You must be signed in to change notification settings - Fork 17.4k
Conversation
This reverts commit 3da827b.
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`.
Builds are green, I ended up canceling https://github.visualstudio.com/Atom/_build/results?buildId=41619 because I think AppVeyor mistakenly thought this was a production branch. Going ahead and merging this. |
I can confirm that the nightly based on Electron 3 works great on Ubuntu :-) |
Just a heads up that Electron did release 3.1.11 |
With Electron v3, shouldn't this issue be fixed: electron/electron#2073 ( atom/src/main-process/atom-application.js Line 285 in 119fead
atom/src/main-process/atom-application.js Line 110 in 119fead
|
Reviewing all Thanks for bringing it up though! |
Thanks for bringing that up @mfonville ! we need to do some verification in order to remove these two TODOs, but it's under our radar 😄 |
Fixes #19372
Previously, we had merged the Electron 3 upgrade branch into master, but quickly discovered several ways in which Atom could crash. That caused us to revert the upgrade so that we could investigate where the problem was occurring.
All of the crashes could ultimately be traced back to
mksnapshot
, which was compiled with a set of flags that would confuse Node once the snapshot was loaded into V8 (see electron/electron#18420). This pull request builds on top of #18916 but uses Electron 3.1.10 instead, which fixes the snapshot issue.Note that, with this pull request, we are also bumping
@atom/nsfw
to v1.0.23. While not technically needed for the Electron upgrade, this new version contains several fixes that had been introduced in the upstream repository but that our fork wasn't yet using. For more information, see atom/nsfw#6.I have been using this branch for 10 days now and, after several stress tests, I couldn't observe the hard crashes anymore. It was relatively easy to reproduce them with the buggy
mksnapshot
, so I am reasonably confident those issues have finally been resolved.This pull request only contains a few minor changes in addition to #18916, so I am planning to merge it to master as soon as we get a green build.