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

fix: isolate callbacks in node_bindings #23261

Merged
merged 2 commits into from
Apr 27, 2020
Merged

fix: isolate callbacks in node_bindings #23261

merged 2 commits into from
Apr 27, 2020

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Apr 23, 2020

Description of Change

While some aspects of embedding are necessarily different between emulated Node.js and Node.js as it's embedded in the main and renderer processes of Electron, there's been some skew recently in our embedding processes resultant of the fact that we only test some of Node's functionality in emulated mode and therefore miss potential holes in non-emulated mode.

In the future, we should seek to test these more effectively, but for now we should at least close that gap by ensuring that isolate setup is better executed in non-emulated mode. There remains some work to be done here around async_hooks (see #23213) but that will require a little more investigation to determine what of SetupIsolateForNode can be safely executed in a non-emulated environment without causing collateral damage.

cc @nornagon @ckerr @MarshallOfSound

Checklist

Release Notes

Notes: Fixed some Wasm and diagnostics issues in main and renderer process execution of Node.js

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Apr 23, 2020
shell/common/node_bindings.cc Outdated Show resolved Hide resolved
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Apr 24, 2020
@zcbenz zcbenz merged commit 979c291 into master Apr 27, 2020
@release-clerk
Copy link

release-clerk bot commented Apr 27, 2020

Release Notes Persisted

Fixed some Wasm and diagnostics issues in main and renderer process execution of Node.js

@zcbenz zcbenz deleted the node-bindings-fixes branch April 27, 2020 08:03
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.

4 participants