diff --git a/src/env.h b/src/env.h index c768120ab8..1e3746b263 100644 --- a/src/env.h +++ b/src/env.h @@ -50,7 +50,8 @@ namespace node { #define PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V) \ V(alpn_buffer_private_symbol, "node:alpnBuffer") \ V(arrow_message_private_symbol, "node:arrowMessage") \ - V(contextify_private_symbol, "node:contextify") \ + V(contextify_context_private_symbol, "node:contextify:context") \ + V(contextify_global_private_symbol, "node:contextify:global") \ V(decorated_private_symbol, "node:decorated") \ V(npn_buffer_private_symbol, "node:npnBuffer") \ V(processed_private_symbol, "node:processed") \ diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 6e04d5836f..4870994789 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -209,7 +209,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->SetPrivate(env->context(), + env->contextify_global_private_symbol(), + ctx->Global()); env->AssignToContext(ctx); @@ -270,7 +280,7 @@ class ContextifyContext { CHECK( !sandbox->HasPrivate( env->context(), - env->contextify_private_symbol()).FromJust()); + env->contextify_context_private_symbol()).FromJust()); TryCatch try_catch(env->isolate()); ContextifyContext* context = new ContextifyContext(env, sandbox); @@ -285,7 +295,7 @@ class ContextifyContext { sandbox->SetPrivate( env->context(), - env->contextify_private_symbol(), + env->contextify_context_private_symbol(), External::New(env->isolate(), context)); } @@ -300,7 +310,8 @@ class ContextifyContext { Local sandbox = args[0].As(); auto result = - sandbox->HasPrivate(env->context(), env->contextify_private_symbol()); + sandbox->HasPrivate(env->context(), + env->contextify_context_private_symbol()); args.GetReturnValue().Set(result.FromJust()); } @@ -315,7 +326,8 @@ class ContextifyContext { Environment* env, const Local& sandbox) { auto maybe_value = - sandbox->GetPrivate(env->context(), env->contextify_private_symbol()); + sandbox->GetPrivate(env->context(), + env->contextify_context_private_symbol()); Local context_external_v; if (maybe_value.ToLocal(&context_external_v) && context_external_v->IsExternal()) { 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 94527598ca..15efc8f527 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.