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

src: split up InitializeContext #30067

Closed

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Oct 22, 2019

Refs electron/electron#20684.
Refs 1ec4154.

This PR splits out code from InitializeContext into a new function InitializeContextForSnapshot and moves the callsite of InitializeContextRuntime from NewContext to InitializeContext - embedders don't necessarily call NewContext and so we experienced a primordial-related crash:

/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Helper (Renderer).app/Contents/MacOS/Electron Helper (Renderer)[69516]: ../../third_party/electron_node/src/env.cc:256:void node::Environment::CreateProperties(): Assertion `primordials->IsObject()' failed.
1: 0x107e8e3c5 node::Abort() [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 2: 0x107e8e14f node::Assert(node::AssertionInfo const&) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 3: 0x107e3527c node::EmitAsyncDestroy(node::Environment*, node::async_context) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 4: 0x107e362dc node::EmitAsyncDestroy(node::Environment*, node::async_context) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 5: 0x107e0d16d node::CreateEnvironment(node::IsolateData*, v8::Local<v8::Context>, int, char const* const*, int, char const* const*, bool) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 6: 0x1024e2b8f AtomInitializeICUandStartNode [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 7: 0x1024f959c AtomInitializeICUandStartNode [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 8: 0x1024f8796 AtomInitializeICUandStartNode [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 9: 0x107cb3048 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
10: 0x1060dc1d5 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
11: 0x1060e2b3a v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
12: 0x1060e2d5a v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
13: 0x106b2d6af v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
14: 0x1024fe179 AtomInitializeICUandStartNode [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
15: 0x107caa7f2 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
16: 0x106a397b0 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
17: 0x106fadee3 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
18: 0x106faa0f0 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
19: 0x106fa9817 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
20: 0x106fac8aa v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
21: 0x106fba636 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
22: 0x106b32cd1 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
23: 0x107ca0ca4 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
24: 0x107d2b1d9 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
25: 0x107d2b06b v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
26: 0x107c9ff25 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
27: 0x107ca0161 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
28: 0x107c7173d v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
29: 0x102a0e9bb AtomInitializeICUandStartNode [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
30: 0x107c86b86 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
31: 0x104a4309b v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
32: 0x104da29b5 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
33: 0x104da2aa3 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
34: 0x10472a2c2 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
35: 0x10473a27a v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
36: 0x10473a707 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
37: 0x104797d23 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
38: 0x1046d2ada v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
39: 0x10479768f v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
40: 0x7fff320ef683 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ [/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation]
41: 0x7fff320ef629 __CFRunLoopDoSource0 [/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation]
42: 0x7fff320d2feb __CFRunLoopDoSources0 [/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation]
43: 0x7fff320d25b5 __CFRunLoopRun [/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation]
44: 0x7fff320d1ebe CFRunLoopRunSpecific [/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation]
45: 0x7fff3433632f -[NSRunLoop(NSRunLoop) runMode:beforeDate:] [/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation]
46: 0x104798231 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
47: 0x104797058 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
48: 0x10473abe2 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
49: 0x10470e9a7 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
50: 0x107cea732 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
51: 0x103dd105c v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
52: 0x10607f6a9 v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*) [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
53: 0x102f6e3c4 AtomInitializeICUandStartNode [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
54: 0x102393194 AtomMain [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
55: 0x101a2b465  [/Users/codebytere/Library/Application Support/Electron Fiddle/electron-bin/8.0.0-nightly.20191021/Electron.app/Contents/Frameworks/Electron Helper (Renderer).app/Contents/MacOS/Electron Helper (Renderer)]
56: 0x7fff5e02b3d5 start [/usr/lib/system/libdyld.dylib]
Received signal 6

InitializeContextRuntime wasn't a public method, so we couldn't call it otherwise. To resolve this,
any initialization logic should be performed in InitializeContext to minimize surface area of potential breakage for embedders like Electron.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 22, 2019
@devsnek
Copy link
Member

devsnek commented Oct 22, 2019

InitializeContextRuntime is separate from InitializeContext on purpose. It should only be called at runtime, but InitializeContext might be called at snapshot time.

@codebytere
Copy link
Member Author

codebytere commented Oct 22, 2019

Then would you prefer i expose this as a new embedder API? Either way, we'll need to find a reusable solution for embedders here I would say.

@devsnek
Copy link
Member

devsnek commented Oct 22, 2019

@codebytere i'm not really sure what the constraints here are. If electron can't call NewContext for some reason it should duplicate the logic I guess.

@MarshallOfSound
Copy link
Member

@devsnek We can't call NewContext because we are not responsible for creating the context, we are given an existing context and we want to give that to node. If there is logic that node must run on a context to make it work, that logic must either be run on the exposed APIs that allow embedders to initialize a context or a new API must be exposed and a breaking change declared for embedders.

Based on a quick code search (and knowing the history of InitializeContext) that method is called in exactly one place. The exact same place that InitializeContextRuntime is called. The initialize method was exposed in #28544 for the sole use case of embedders initializing an already created context with node's special logic.

I'm not sure I understand the snapshot reasoning when InitializeContextRuntime is called whenever InitializeContext is called.

Duplicating the logic doesn't make much sense either as that is again a breaking change for embedders and it means that all embedders that use the Initialize API suddenly need to start duplicating logic (some of which may use node internals that can't be called without further duplication).

@devsnek
Copy link
Member

devsnek commented Oct 22, 2019

@MarshallOfSound it should be called at runtime whenever a node context is created or loaded from a snapshot.

src/api/environment.cc Outdated Show resolved Hide resolved
@addaleax
Copy link
Member

Maybe for context – I have wondered the same thing as you, @MarshallOfSound; the reason for this separation is, as I understand the explanation that @devsnek gave me, that snapshot dons’t actually reflect removals from e.g. builtin objects, like the one we perform with Atomics.wake.

I feel like a comment in the code about that would be a good idea. And if necessary, exposing this as an embedder method seems fine to me.

@devsnek
Copy link
Member

devsnek commented Oct 22, 2019

@addaleax fwiw, the Atomics global isn't present at all during snapshot capture.

@MarshallOfSound
Copy link
Member

Sorry if I'm missing something, but this separation still makes no sense. From nodes perspective, there is 0 difference from before this PR and after. The same code runs, in the same order.

From Electron (Embedders) perspective, what was broken, now works again 🤷‍♂

What am I missing? This PR fixes embedders, and has no impact on node?

@joyeecheung
Copy link
Member

joyeecheung commented Oct 22, 2019

From the perspective of an embedder I don't think any new API needs to be introduced for now - we don't provide any proper hooks for embedders to do anything about the snapshots and that need to be well thought-out than just an ad-hoc InitializeContextRuntime. Having just InitializeContext which runs through an existing context to do everything is sufficient and backwards compatible.

From the perspective of Node.js internals I think it makes sense to rename the internal InitializeContext to something like InitializeContextForSnapshot, and then call both InitializeContextForSnapshot and InitializeContextRuntime in the public InitializeContext. So then NewContext is essentially Context::New + InitializeContext, and embedders can choose to just call InitializeContext on an existing context as usual.

@devsnek
Copy link
Member

devsnek commented Oct 23, 2019

@joyeecheung i think that sounds okay. the only thing i want to make sure of is that InitializeContextRuntime isn't called twice on a context, not because i think it will break anything at the moment, but it could cause trouble in the future...

@joyeecheung
Copy link
Member

joyeecheung commented Oct 23, 2019

@devsnek AFAICT InitializeContextRuntime is just a temporary ad-hoc internal helper which contains two tasks that are also temporary ad-hoc fixes (it's hard to tell who may disappear first, the helper or the fixes) so I would not worry too much about future footguns.

@codebytere
Copy link
Member Author

@joyeecheung so you mean pull the existing code in InitializeContext into a new method InitializeContextForSnapshot and then have the public one call into that as well as the existing InitializeContextRuntime?

@joyeecheung
Copy link
Member

@codebytere Yes

@codebytere
Copy link
Member Author

Should be set! Thanks for the guidance :)

@codebytere codebytere changed the title src: move InitializeContextRuntime to InitializeContext src: split up InitializeContext Oct 23, 2019
@nodejs-github-bot
Copy link
Collaborator

@danbev
Copy link
Contributor

danbev commented Oct 25, 2019

Re-run of failing node-test-commit-windows-fanned (✔️)

codebytere added a commit that referenced this pull request Oct 29, 2019
This splits out code from InitializeContext into a
new function InitializeContextForSnapshot and
moves the callsite of InitializeContextRuntime from
NewContext to InitializeContext - embedders don't
necessarily call NewContext and so need to be able
to guarantee these functions are called regardless.

PR-URL: #30067
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@codebytere
Copy link
Member Author

Landed in b086e38

@codebytere codebytere closed this Oct 29, 2019
@codebytere codebytere deleted the move-initializecontextruntime branch October 29, 2019 15:15
targos pushed a commit that referenced this pull request Nov 5, 2019
This splits out code from InitializeContext into a
new function InitializeContextForSnapshot and
moves the callsite of InitializeContextRuntime from
NewContext to InitializeContext - embedders don't
necessarily call NewContext and so need to be able
to guarantee these functions are called regardless.

PR-URL: #30067
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@targos targos mentioned this pull request Nov 5, 2019
targos pushed a commit that referenced this pull request Nov 8, 2019
This splits out code from InitializeContext into a
new function InitializeContextForSnapshot and
moves the callsite of InitializeContextRuntime from
NewContext to InitializeContext - embedders don't
necessarily call NewContext and so need to be able
to guarantee these functions are called regardless.

PR-URL: #30067
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Nov 10, 2019
This splits out code from InitializeContext into a
new function InitializeContextForSnapshot and
moves the callsite of InitializeContextRuntime from
NewContext to InitializeContext - embedders don't
necessarily call NewContext and so need to be able
to guarantee these functions are called regardless.

PR-URL: #30067
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Nov 10, 2019
This splits out code from InitializeContext into a
new function InitializeContextForSnapshot and
moves the callsite of InitializeContextRuntime from
NewContext to InitializeContext - embedders don't
necessarily call NewContext and so need to be able
to guarantee these functions are called regardless.

PR-URL: #30067
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Nov 11, 2019
This splits out code from InitializeContext into a
new function InitializeContextForSnapshot and
moves the callsite of InitializeContextRuntime from
NewContext to InitializeContext - embedders don't
necessarily call NewContext and so need to be able
to guarantee these functions are called regardless.

PR-URL: #30067
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.