Skip to content

Commit

Permalink
contextify: tie lifetimes of context & sandbox
Browse files Browse the repository at this point in the history
When the previous set of changes (bfff07b) it was possible to have the
context get garbage collected while sandbox was still live. We need to
tie the lifetime of the context to the lifetime of the sandbox.

This is a backport of nodejs#5786 to v5.x.

Ref: nodejs#5786
PR-URL: nodejs#5800
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
ofrobots authored and rvagg committed May 20, 2016
1 parent 4d48400 commit c6db822
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
10 changes: 10 additions & 0 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,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);

Expand Down
9 changes: 9 additions & 0 deletions test/parallel/test-vm-create-and-run-in-context.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
'use strict';
// Flags: --expose-gc
require('../common');
var assert = require('assert');

Expand All @@ -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.

0 comments on commit c6db822

Please sign in to comment.