1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

src: make cross-context MakeCallback() calls work

Browse Source 
Check that invoking a callback on a receiver from a different context
works.

It ran afoul of an `env->context() == isolate->GetCurrentContext()`
assertion so retrieve the environment from the callback context and
the context to enter from the environment's context() method.

We could also have retrieved the environment from the receiver's context
and that would have made little practical difference.  It just seemed
more correct to get it from the callback context because that is the
actual execution context.

PR-URL: https://github.com/nodejs/node/pull/9221
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit is contained in:
Ben Noordhuis 2016-10-21 11:57:20 +02:00
parent e4290dec2d
commit 921d2b080e
2 changed files with 34 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
src/node.cc
View File

@ -1304,45 +1304,48 @@ Local<Value> MakeCallback(Environment* env,
Local<Value> MakeCallback(Isolate* isolate, Local<Value> MakeCallback(Isolate* isolate,
Local<Object> recv, Local<Object> recv,
const char* method, const char* method,
int argc, int argc,
Local<Value> argv[]) { Local<Value> argv[]) {
EscapableHandleScope handle_scope(isolate); EscapableHandleScope handle_scope(isolate);
Local<Context> context = recv->CreationContext(); Local<String> method_string = OneByteString(isolate, method);
Environment* env = Environment::GetCurrent(context);
Context::Scope context_scope(context);
return handle_scope.Escape( return handle_scope.Escape(
Local<Value>::New(isolate, MakeCallback(env, recv, method, argc, argv))); MakeCallback(isolate, recv, method_string, argc, argv));
} }
Local<Value> MakeCallback(Isolate* isolate, Local<Value> MakeCallback(Isolate* isolate,
Local<Object> recv, Local<Object> recv,
Local<String> symbol, Local<String> symbol,
int argc, int argc,
Local<Value> argv[]) { Local<Value> argv[]) {
EscapableHandleScope handle_scope(isolate); EscapableHandleScope handle_scope(isolate);
Local<Context> context = recv->CreationContext(); Local<Value> callback_v = recv->Get(symbol);
Environment* env = Environment::GetCurrent(context); if (callback_v.IsEmpty()) return Local<Value>();
Context::Scope context_scope(context); if (!callback_v->IsFunction()) return Local<Value>();
return handle_scope.Escape( Local<Function> callback = callback_v.As<Function>();
Local<Value>::New(isolate, MakeCallback(env, recv, symbol, argc, argv))); return handle_scope.Escape(MakeCallback(isolate, recv, callback, argc, argv));
} }
Local<Value> MakeCallback(Isolate* isolate, Local<Value> MakeCallback(Isolate* isolate,
Local<Object> recv, Local<Object> recv,
Local<Function> callback, Local<Function> callback,
int argc, int argc,
Local<Value> argv[]) { Local<Value> argv[]) {
// Observe the following two subtleties:
//
// 1. The environment is retrieved from the callback function's context.
// 2. The context to enter is retrieved from the environment.
//
// Because of the AssignToContext() call in src/node_contextify.cc,
// the two contexts need not be the same.
EscapableHandleScope handle_scope(isolate); EscapableHandleScope handle_scope(isolate);
Local<Context> context = recv->CreationContext(); Environment* env = Environment::GetCurrent(callback->CreationContext());
Environment* env = Environment::GetCurrent(context); Context::Scope context_scope(env->context());
Context::Scope context_scope(context); return handle_scope.Escape(
return handle_scope.Escape(Local<Value>::New( MakeCallback(env, recv.As<Value>(), callback, argc, argv));
isolate,
MakeCallback(env, recv.As<Value>(), callback, argc, argv)));
} }

4
test/addons/make-callback/test.js
View File

@ -36,6 +36,10 @@ const recv = {
assert.strictEqual(42, makeCallback(recv, 'one')); assert.strictEqual(42, makeCallback(recv, 'one'));
assert.strictEqual(42, makeCallback(recv, 'two', 1337)); assert.strictEqual(42, makeCallback(recv, 'two', 1337));
// Check that callbacks on a receiver from a different context works.
const foreignObject = vm.runInNewContext('({ fortytwo() { return 42; } })');
assert.strictEqual(42, makeCallback(foreignObject, 'fortytwo'));
// Check that the callback is made in the context of the receiver. // Check that the callback is made in the context of the receiver.
const target = vm.runInNewContext(` const target = vm.runInNewContext(`
(function($Object) { (function($Object) {