diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 59a90c88c9993f..8235dcd49e093c 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -43,53 +43,33 @@ using v8::TryCatch; using v8::UnboundScript; using v8::V8; using v8::Value; -using v8::WeakCallbackData; +using v8::WeakCallbackInfo; class ContextifyContext { protected: - enum Kind { - kSandbox, - kContext, - kProxyGlobal - }; + // V8 reserves the first field in context objects for the debugger. We use the + // second field to hold a reference to the sandbox object. + enum { kSandboxObjectIndex = 1 }; Environment* const env_; - Persistent sandbox_; Persistent context_; - Persistent proxy_global_; - int references_; public: - explicit ContextifyContext(Environment* env, Local sandbox) - : env_(env), - sandbox_(env->isolate(), sandbox), - // Wait for sandbox_, proxy_global_, and context_ to die - references_(0) { - context_.Reset(env->isolate(), CreateV8Context(env)); - - sandbox_.SetWeak(this, WeakCallback); - sandbox_.MarkIndependent(); - references_++; + ContextifyContext(Environment* env, Local sandbox_obj) : env_(env) { + Local v8_context = CreateV8Context(env, sandbox_obj); + context_.Reset(env->isolate(), v8_context); // Allocation failure or maximum call stack size reached if (context_.IsEmpty()) return; - context_.SetWeak(this, WeakCallback); + context_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter); context_.MarkIndependent(); - references_++; - - proxy_global_.Reset(env->isolate(), context()->Global()); - proxy_global_.SetWeak(this, WeakCallback); - proxy_global_.MarkIndependent(); - references_++; } ~ContextifyContext() { context_.Reset(); - proxy_global_.Reset(); - sandbox_.Reset(); } @@ -103,6 +83,15 @@ class ContextifyContext { } + inline Local global_proxy() const { + return context()->Global(); + } + + + inline Local sandbox() const { + return Local::Cast(context()->GetEmbedderData(kSandboxObjectIndex)); + } + // XXX(isaacs): This function only exists because of a shortcoming of // the V8 SetNamedPropertyHandler function. // @@ -130,7 +119,7 @@ class ContextifyContext { Local context = PersistentToLocal(env()->isolate(), context_); Local global = context->Global()->GetPrototype()->ToObject(env()->isolate()); - Local sandbox = PersistentToLocal(env()->isolate(), sandbox_); + Local sandbox_obj = sandbox(); Local clone_property_method; @@ -138,7 +127,7 @@ class ContextifyContext { int length = names->Length(); for (int i = 0; i < length; i++) { Local key = names->Get(i)->ToString(env()->isolate()); - bool has = sandbox->HasOwnProperty(key); + bool has = sandbox_obj->HasOwnProperty(context, key).FromJust(); if (!has) { // Could also do this like so: // @@ -171,7 +160,7 @@ class ContextifyContext { clone_property_method = Local::Cast(script->Run()); CHECK(clone_property_method->IsFunction()); } - Local args[] = { global, key, sandbox }; + Local args[] = { global, key, sandbox_obj }; clone_property_method->Call(global, ARRAY_SIZE(args), args); } } @@ -195,14 +184,13 @@ class ContextifyContext { } - Local CreateV8Context(Environment* env) { + Local CreateV8Context(Environment* env, Local sandbox_obj) { EscapableHandleScope scope(env->isolate()); Local function_template = FunctionTemplate::New(env->isolate()); function_template->SetHiddenPrototype(true); - Local sandbox = PersistentToLocal(env->isolate(), sandbox_); - function_template->SetClassName(sandbox->GetConstructorName()); + function_template->SetClassName(sandbox_obj->GetConstructorName()); Local object_template = function_template->InstanceTemplate(); @@ -220,6 +208,17 @@ class ContextifyContext { CHECK(!ctx.IsEmpty()); ctx->SetSecurityToken(env->context()->GetSecurityToken()); + // We need to tie the lifetime of the sandbox object with the lifetime of + // newly created context. We do this by making them hold references to each + // other. The context can directly hold a reference to the sandbox as an + // embedder data field. However, we cannot hold a reference to a v8::Context + // directly in an Object, we instead hold onto the new context's global + // object instead (which then has a reference to the context). + ctx->SetEmbedderData(kSandboxObjectIndex, sandbox_obj); + sandbox_obj->SetHiddenValue( + FIXED_ONE_BYTE_STRING(env->isolate(), "_contextifyHiddenGlobal"), + ctx->Global()); + env->AssignToContext(ctx); return scope.Escape(ctx); @@ -313,18 +312,9 @@ class ContextifyContext { } - template - static void WeakCallback(const WeakCallbackData& data) { + static void WeakCallback(const WeakCallbackInfo& data) { ContextifyContext* context = data.GetParameter(); - if (kind == kSandbox) - context->sandbox_.ClearWeak(); - else if (kind == kContext) - context->context_.ClearWeak(); - else - context->proxy_global_.ClearWeak(); - - if (--context->references_ == 0) - delete context; + delete context; } @@ -346,8 +336,6 @@ class ContextifyContext { static void GlobalPropertyGetterCallback( Local property, const PropertyCallbackInfo& args) { - Isolate* isolate = args.GetIsolate(); - ContextifyContext* ctx = Unwrap(args.Data().As()); @@ -355,19 +343,19 @@ class ContextifyContext { if (ctx->context_.IsEmpty()) return; - Local sandbox = PersistentToLocal(isolate, ctx->sandbox_); + Local context = ctx->context(); + Local sandbox = ctx->sandbox(); MaybeLocal maybe_rv = - sandbox->GetRealNamedProperty(ctx->context(), property); + sandbox->GetRealNamedProperty(context, property); if (maybe_rv.IsEmpty()) { - Local proxy_global = PersistentToLocal(isolate, - ctx->proxy_global_); - maybe_rv = proxy_global->GetRealNamedProperty(ctx->context(), property); + maybe_rv = + ctx->global_proxy()->GetRealNamedProperty(context, property); } Local rv; if (maybe_rv.ToLocal(&rv)) { - if (rv == ctx->sandbox_) - rv = PersistentToLocal(isolate, ctx->proxy_global_); + if (rv == sandbox) + rv = ctx->global_proxy(); args.GetReturnValue().Set(rv); } @@ -378,8 +366,6 @@ class ContextifyContext { Local property, Local value, const PropertyCallbackInfo& args) { - Isolate* isolate = args.GetIsolate(); - ContextifyContext* ctx = Unwrap(args.Data().As()); @@ -387,15 +373,13 @@ class ContextifyContext { if (ctx->context_.IsEmpty()) return; - PersistentToLocal(isolate, ctx->sandbox_)->Set(property, value); + ctx->sandbox()->Set(property, value); } static void GlobalPropertyQueryCallback( Local property, const PropertyCallbackInfo& args) { - Isolate* isolate = args.GetIsolate(); - ContextifyContext* ctx = Unwrap(args.Data().As()); @@ -403,17 +387,14 @@ class ContextifyContext { if (ctx->context_.IsEmpty()) return; - Local sandbox = PersistentToLocal(isolate, ctx->sandbox_); + Local context = ctx->context(); Maybe maybe_prop_attr = - sandbox->GetRealNamedPropertyAttributes(ctx->context(), property); + ctx->sandbox()->GetRealNamedPropertyAttributes(context, property); if (maybe_prop_attr.IsNothing()) { - Local proxy_global = PersistentToLocal(isolate, - ctx->proxy_global_); - maybe_prop_attr = - proxy_global->GetRealNamedPropertyAttributes(ctx->context(), - property); + ctx->global_proxy()->GetRealNamedPropertyAttributes(context, + property); } if (maybe_prop_attr.IsJust()) { @@ -426,8 +407,6 @@ class ContextifyContext { static void GlobalPropertyDeleterCallback( Local property, const PropertyCallbackInfo& args) { - Isolate* isolate = args.GetIsolate(); - ContextifyContext* ctx = Unwrap(args.Data().As()); @@ -435,9 +414,7 @@ class ContextifyContext { if (ctx->context_.IsEmpty()) return; - Local sandbox = PersistentToLocal(isolate, ctx->sandbox_); - - Maybe success = sandbox->Delete(ctx->context(), property); + Maybe success = ctx->sandbox()->Delete(ctx->context(), property); if (success.IsJust()) args.GetReturnValue().Set(success.FromJust()); @@ -453,8 +430,7 @@ class ContextifyContext { if (ctx->context_.IsEmpty()) return; - Local sandbox = PersistentToLocal(args.GetIsolate(), ctx->sandbox_); - args.GetReturnValue().Set(sandbox->GetPropertyNames()); + args.GetReturnValue().Set(ctx->sandbox()->GetPropertyNames()); } }; diff --git a/test/parallel/test-vm-create-and-run-in-context.js b/test/parallel/test-vm-create-and-run-in-context.js index 94527598ca27b1..15efc8f527b8c9 100644 --- a/test/parallel/test-vm-create-and-run-in-context.js +++ b/test/parallel/test-vm-create-and-run-in-context.js @@ -1,4 +1,5 @@ 'use strict'; +// Flags: --expose-gc require('../common'); var assert = require('assert'); @@ -18,3 +19,11 @@ console.error('test updating context'); result = vm.runInContext('var foo = 3;', context); assert.equal(3, context.foo); assert.equal('lala', context.thing); + +// /~https://github.com/nodejs/node/issues/5768 +console.error('run in contextified sandbox without referencing the context'); +var sandbox = {x: 1}; +vm.createContext(sandbox); +gc(); +vm.runInContext('x = 2', sandbox); +// Should not crash.