This repository has been archived by the owner on Oct 15, 2020. It is now read-only.
deps,src: workaround vm.runInContext issue with top level "var" #542
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looks like OSX failed due to running out of disk. |
|
Looks like there is a node test |
kfarnung
reviewed
May 21, 2018
| void CacheGlobalProperties(); | ||
| void ResolveGlobalChanges(JsValueRef sandbox); | ||
|
|
||
|
|
kfarnung
reviewed
May 21, 2018
deps/chakrashim/lib/chakra_shim.js
Outdated
| return true; | ||
| } | ||
| utils.afterContext = function (beforeDescriptors, contextGlobal, sandbox) { | ||
| try{ |
There was a problem hiding this comment.
nit: spacing around braces. I think the linter on Windows should have caught it.
MSLaguana
force-pushed
the
vmContextWorkaround
branch
from
e610a21 to
e772faf
Compare
MSLaguana
added a commit
to MSLaguana/node-chakracore
that referenced
this pull request
May 21, 2018
addresses nodejs#420 ChakraCore doesn't handle global objects in the same way as v8, and in particular ChakraCore doesn't support all proxy interceptors there. We attempted to work around this via interceptors in the prototype chain, but that was an imperfect solution, and a top-level "var" declaration would end up adding a property to the global object in a context without going through the interceptors. To work around this in simple cases, we now take a snapshot of properties on the global before running in a context, and diff the changes at the end to patch up changes in the sandbox. This is NOT a complete solution, and in particular will not work if there is any sort of asynchronous operation that changes the state of the global object. It also has behavioral differences if the global object is writable but the sandbox object is not; in node-v8 this will result in the variable silently not being set in the context and instead the value from the sandbox being used, while here we discover the mismatch too late and the new value has been set on the global. PR-URL: nodejs#542 Reviewed-By: Seth Brenith <sethb@microsoft.com> Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
addresses nodejs#420 ChakraCore doesn't handle global objects in the same way as v8, and in particular ChakraCore doesn't support all proxy interceptors there. We attempted to work around this via interceptors in the prototype chain, but that was an imperfect solution, and a top-level "var" declaration would end up adding a property to the global object in a context without going through the interceptors. To work around this in simple cases, we now take a snapshot of properties on the global before running in a context, and diff the changes at the end to patch up changes in the sandbox. This is NOT a complete solution, and in particular will not work if there is any sort of asynchronous operation that changes the state of the global object. It also has behavioral differences if the global object is writable but the sandbox object is not; in node-v8 this will result in the variable silently not being set in the context and instead the value from the sandbox being used, while here we discover the mismatch too late and the new value has been set on the global. PR-URL: nodejs#542 Reviewed-By: Seth Brenith <sethb@microsoft.com> Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
MSLaguana
force-pushed
the
vmContextWorkaround
branch
from
e772faf to
e7643d2
Compare
kfarnung
approved these changes
May 21, 2018
kfarnung
pushed a commit
to kfarnung/node-chakracore
that referenced
this pull request
May 22, 2018
addresses nodejs#420 ChakraCore doesn't handle global objects in the same way as v8, and in particular ChakraCore doesn't support all proxy interceptors there. We attempted to work around this via interceptors in the prototype chain, but that was an imperfect solution, and a top-level "var" declaration would end up adding a property to the global object in a context without going through the interceptors. To work around this in simple cases, we now take a snapshot of properties on the global before running in a context, and diff the changes at the end to patch up changes in the sandbox. This is NOT a complete solution, and in particular will not work if there is any sort of asynchronous operation that changes the state of the global object. It also has behavioral differences if the global object is writable but the sandbox object is not; in node-v8 this will result in the variable silently not being set in the context and instead the value from the sandbox being used, while here we discover the mismatch too late and the new value has been set on the global. PR-URL: nodejs#542 Reviewed-By: Seth Brenith <sethb@microsoft.com> Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
kfarnung
pushed a commit
to kfarnung/node-chakracore
that referenced
this pull request
May 22, 2018
addresses nodejs#420 ChakraCore doesn't handle global objects in the same way as v8, and in particular ChakraCore doesn't support all proxy interceptors there. We attempted to work around this via interceptors in the prototype chain, but that was an imperfect solution, and a top-level "var" declaration would end up adding a property to the global object in a context without going through the interceptors. To work around this in simple cases, we now take a snapshot of properties on the global before running in a context, and diff the changes at the end to patch up changes in the sandbox. This is NOT a complete solution, and in particular will not work if there is any sort of asynchronous operation that changes the state of the global object. It also has behavioral differences if the global object is writable but the sandbox object is not; in node-v8 this will result in the variable silently not being set in the context and instead the value from the sandbox being used, while here we discover the mismatch too late and the new value has been set on the global. PR-URL: nodejs#542 Reviewed-By: Seth Brenith <sethb@microsoft.com> Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
addresses #420
ChakraCore doesn't handle global objects in the same way as v8, and
in particular ChakraCore doesn't support all proxy interceptors there.
We attempted to work around this via interceptors in the prototype
chain, but that was an imperfect solution, and a top-level "var"
declaration would end up adding a property to the global object in
a context without going through the interceptors.
To work around this in simple cases, we now take a snapshot of
properties on the global before running in a context, and diff the
changes at the end to patch up changes in the sandbox.
This is NOT a complete solution, and in particular will not work if
there is any sort of asynchronous operation that changes the state
of the global object. It also has behavioral differences if the
global object is writable but the sandbox object is not; in node-v8
this will result in the variable silently not being set in the
context and instead the value from the sandbox being used, while
here we discover the mismatch too late and the new value has been
set on the global.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes