Skip to content

Commit

Permalink
contextify: cleanup weak ref for sandbox
Browse files Browse the repository at this point in the history 
Simplify how node_contextify was keeping a weak reference to the
sandbox object in order to prepare for new style phantom weakness V8
API. It is simpler (and more robust) for the context to hold a
reference to the sandbox in an embedder data field. Doing otherwise
meant that the sandbox could become weak while the context was still
alive. This wasn't a problem because we would make the reference
strong at that point.

Since the sandbox must live at least as long as the context, it
would be better for the context to hold onto the sandbox.

PR-URL: nodejs#5392
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
@ofrobots @rvagg
ofrobots authored and rvagg committed May 20, 2016
1 parent faa94ff commit 3d56671
Showing 1 changed file with 28 additions and 52 deletions.
80 changes: 28 additions & 52 deletions src/node_contextify.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,40 +48,29 @@ using v8::WeakCallbackData;

class ContextifyContext {
protected:
enum Kind {
kSandbox,
kContext
};
// 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<Object> sandbox_;
Persistent<Context> context_;
int references_;

public:
explicit ContextifyContext(Environment* env, Local<Object> sandbox)
: env_(env),
sandbox_(env->isolate(), sandbox),
// Wait for sandbox_ and context_ to die
references_(0) {
context_.Reset(env->isolate(), CreateV8Context(env));

sandbox_.SetWeak(this, WeakCallback<Object, kSandbox>);
sandbox_.MarkIndependent();
references_++;
explicit ContextifyContext(Environment* env, Local<Object> sandbox_obj)
: env_(env) {
Local<Context> 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, kContext>);
context_.SetWeak(this, WeakCallback<Context>);
context_.MarkIndependent();
references_++;
}


~ContextifyContext() {
context_.Reset();
sandbox_.Reset();
}


Expand All @@ -99,6 +88,11 @@ class ContextifyContext {
return context()->Global();
}


inline Local<Object> sandbox() const {
return Local<Object>::Cast(context()->GetEmbedderData(kSandboxObjectIndex));
}

// XXX(isaacs): This function only exists because of a shortcoming of
// the V8 SetNamedPropertyHandler function.
//
Expand Down Expand Up @@ -126,15 +120,14 @@ class ContextifyContext {
Local<Context> context = PersistentToLocal(env()->isolate(), context_);
Local<Object> global =
context->Global()->GetPrototype()->ToObject(env()->isolate());
Local<Object> sandbox = PersistentToLocal(env()->isolate(), sandbox_);

Local<Function> clone_property_method;

Local<Array> names = global->GetOwnPropertyNames();
int length = names->Length();
for (int i = 0; i < length; i++) {
Local<String> key = names->Get(i)->ToString(env()->isolate());
bool has = sandbox->HasOwnProperty(key);
bool has = sandbox()->HasOwnProperty(context, key).FromJust();
if (!has) {
// Could also do this like so:
//
Expand Down Expand Up @@ -167,7 +160,7 @@ class ContextifyContext {
clone_property_method = Local<Function>::Cast(script->Run());
CHECK(clone_property_method->IsFunction());
}
Local<Value> args[] = { global, key, sandbox };
Local<Value> args[] = { global, key, sandbox() };
clone_property_method->Call(global, ARRAY_SIZE(args), args);
}
}
Expand All @@ -191,14 +184,13 @@ class ContextifyContext {
}


Local<Context> CreateV8Context(Environment* env) {
Local<Context> CreateV8Context(Environment* env, Local<Object> sandbox_obj) {
EscapableHandleScope scope(env->isolate());
Local<FunctionTemplate> function_template =
FunctionTemplate::New(env->isolate());
function_template->SetHiddenPrototype(true);

Local<Object> sandbox = PersistentToLocal(env->isolate(), sandbox_);
function_template->SetClassName(sandbox->GetConstructorName());
function_template->SetClassName(sandbox_obj->GetConstructorName());

Local<ObjectTemplate> object_template =
function_template->InstanceTemplate();
Expand All @@ -215,6 +207,7 @@ class ContextifyContext {

CHECK(!ctx.IsEmpty());
ctx->SetSecurityToken(env->context()->GetSecurityToken());
ctx->SetEmbedderData(kSandboxObjectIndex, sandbox_obj);

env->AssignToContext(ctx);

Expand Down Expand Up @@ -309,16 +302,11 @@ class ContextifyContext {
}


template <class T, Kind kind>
template <class T>
static void WeakCallback(const WeakCallbackData<T, ContextifyContext>& data) {
ContextifyContext* context = data.GetParameter();
if (kind == kSandbox)
context->sandbox_.ClearWeak();
else
context->context_.ClearWeak();

if (--context->references_ == 0)
delete context;
context->context_.ClearWeak();
delete context;
}


Expand All @@ -340,26 +328,23 @@ class ContextifyContext {
static void GlobalPropertyGetterCallback(
Local<Name> property,
const PropertyCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();

ContextifyContext* ctx =
Unwrap<ContextifyContext>(args.Data().As<Object>());

// Stil initializing
if (ctx->context_.IsEmpty())
return;

Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);
MaybeLocal<Value> maybe_rv =
sandbox->GetRealNamedProperty(ctx->context(), property);
ctx->sandbox()->GetRealNamedProperty(ctx->context(), property);
if (maybe_rv.IsEmpty()) {
maybe_rv =
ctx->global_proxy()->GetRealNamedProperty(ctx->context(), property);
}

Local<Value> rv;
if (maybe_rv.ToLocal(&rv)) {
if (rv == ctx->sandbox_)
if (rv == ctx->sandbox())
rv = ctx->global_proxy();

args.GetReturnValue().Set(rv);
Expand All @@ -371,34 +356,30 @@ class ContextifyContext {
Local<Name> property,
Local<Value> value,
const PropertyCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();

ContextifyContext* ctx =
Unwrap<ContextifyContext>(args.Data().As<Object>());

// Stil initializing
if (ctx->context_.IsEmpty())
return;

PersistentToLocal(isolate, ctx->sandbox_)->Set(property, value);
ctx->sandbox()->Set(property, value);
}


static void GlobalPropertyQueryCallback(
Local<Name> property,
const PropertyCallbackInfo<Integer>& args) {
Isolate* isolate = args.GetIsolate();

ContextifyContext* ctx =
Unwrap<ContextifyContext>(args.Data().As<Object>());

// Stil initializing
if (ctx->context_.IsEmpty())
return;

Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);
Maybe<PropertyAttribute> maybe_prop_attr =
sandbox->GetRealNamedPropertyAttributes(ctx->context(), property);
ctx->sandbox()->GetRealNamedPropertyAttributes(ctx->context(),
property);

if (maybe_prop_attr.IsNothing()) {
maybe_prop_attr =
Expand All @@ -416,18 +397,14 @@ class ContextifyContext {
static void GlobalPropertyDeleterCallback(
Local<Name> property,
const PropertyCallbackInfo<Boolean>& args) {
Isolate* isolate = args.GetIsolate();

ContextifyContext* ctx =
Unwrap<ContextifyContext>(args.Data().As<Object>());

// Stil initializing
if (ctx->context_.IsEmpty())
return;

Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);

Maybe<bool> success = sandbox->Delete(ctx->context(), property);
Maybe<bool> success = ctx->sandbox()->Delete(ctx->context(), property);

if (success.IsJust())
args.GetReturnValue().Set(success.FromJust());
Expand All @@ -443,8 +420,7 @@ class ContextifyContext {
if (ctx->context_.IsEmpty())
return;

Local<Object> sandbox = PersistentToLocal(args.GetIsolate(), ctx->sandbox_);
args.GetReturnValue().Set(sandbox->GetPropertyNames());
args.GetReturnValue().Set(ctx->sandbox()->GetPropertyNames());
}
};

Expand Down

0 comments on commit 3d56671

Please sign in to comment.