FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

contextify: cleanup weak ref for sandbox · nodejs/node@bfff07b · GitHub

/ node Public

Commit bfff07b

Browse files
ofrobots
authored and
rvagg
committed
contextify: cleanup weak ref for sandbox
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: #5392 Reviewed-By: Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
1 parent 93f60cd commit bfff07b

1 file changed

Lines changed: 28 additions & 52 deletions

File tree

‎src/node_contextify.cc‎

Lines changed: 28 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -50,40 +50,29 @@ using v8::WeakCallbackData;
5050

5151
class ContextifyContext {
5252
protected:
53-
enum Kind {
54-
kSandbox,
55-
kContext
56-
};
53+
// V8 reserves the first field in context objects for the debugger. We use the
54+
// second field to hold a reference to the sandbox object.
55+
enum { kSandboxObjectIndex = 1 };
5756

5857
Environment* const env_;
59-
Persistent<Object> sandbox_;
6058
Persistent<Context> context_;
61-
int references_;
6259

6360
public:
64-
explicit ContextifyContext(Environment* env, Local<Object> sandbox)
65-
: env_(env),
66-
sandbox_(env->isolate(), sandbox),
67-
// Wait for sandbox_ and context_ to die
68-
references_(0) {
69-
context_.Reset(env->isolate(), CreateV8Context(env));
70-
71-
sandbox_.SetWeak(this, WeakCallback<Object, kSandbox>);
72-
sandbox_.MarkIndependent();
73-
references_++;
61+
explicit ContextifyContext(Environment* env, Local<Object> sandbox_obj)
62+
: env_(env) {
63+
Local<Context> v8_context = CreateV8Context(env, sandbox_obj);
64+
context_.Reset(env->isolate(), v8_context);
7465

7566
// Allocation failure or maximum call stack size reached
7667
if (context_.IsEmpty())
7768
return;
78-
context_.SetWeak(this, WeakCallback<Context, kContext>);
69+
context_.SetWeak(this, WeakCallback<Context>);
7970
context_.MarkIndependent();
80-
references_++;
8171
}
8272

8373

8474
~ContextifyContext() {
8575
context_.Reset();
86-
sandbox_.Reset();
8776
}
8877

8978

@@ -101,6 +90,11 @@ class ContextifyContext {
10190
return context()->Global();
10291
}
10392

93+
94+
inline Local<Object> sandbox() const {
95+
return Local<Object>::Cast(context()->GetEmbedderData(kSandboxObjectIndex));
96+
}
97+
10498
// XXX(isaacs): This function only exists because of a shortcoming of
10599
// the V8 SetNamedPropertyHandler function.
106100
//
@@ -128,15 +122,14 @@ class ContextifyContext {
128122
Local<Context> context = PersistentToLocal(env()->isolate(), context_);
129123
Local<Object> global =
130124
context->Global()->GetPrototype()->ToObject(env()->isolate());
131-
Local<Object> sandbox = PersistentToLocal(env()->isolate(), sandbox_);
132125

133126
Local<Function> clone_property_method;
134127

135128
Local<Array> names = global->GetOwnPropertyNames();
136129
int length = names->Length();
137130
for (int i = 0; i < length; i++) {
138131
Local<String> key = names->Get(i)->ToString(env()->isolate());
139-
bool has = sandbox->HasOwnProperty(key);
132+
bool has = sandbox()->HasOwnProperty(context, key).FromJust();
140133
if (!has) {
141134
// Could also do this like so:
142135
//
@@ -169,7 +162,7 @@ class ContextifyContext {
169162
clone_property_method = Local<Function>::Cast(script->Run());
170163
CHECK(clone_property_method->IsFunction());
171164
}
172-
Local<Value> args[] = { global, key, sandbox };
165+
Local<Value> args[] = { global, key, sandbox() };
173166
clone_property_method->Call(global, ARRAY_SIZE(args), args);
174167
}
175168
}
@@ -193,14 +186,13 @@ class ContextifyContext {
193186
}
194187

195188

196-
Local<Context> CreateV8Context(Environment* env) {
189+
Local<Context> CreateV8Context(Environment* env, Local<Object> sandbox_obj) {
197190
EscapableHandleScope scope(env->isolate());
198191
Local<FunctionTemplate> function_template =
199192
FunctionTemplate::New(env->isolate());
200193
function_template->SetHiddenPrototype(true);
201194

202-
Local<Object> sandbox = PersistentToLocal(env->isolate(), sandbox_);
203-
function_template->SetClassName(sandbox->GetConstructorName());
195+
function_template->SetClassName(sandbox_obj->GetConstructorName());
204196

205197
Local<ObjectTemplate> object_template =
206198
function_template->InstanceTemplate();
@@ -217,6 +209,7 @@ class ContextifyContext {
217209

218210
CHECK(!ctx.IsEmpty());
219211
ctx->SetSecurityToken(env->context()->GetSecurityToken());
212+
ctx->SetEmbedderData(kSandboxObjectIndex, sandbox_obj);
220213

221214
env->AssignToContext(ctx);
222215

@@ -311,16 +304,11 @@ class ContextifyContext {
311304
}
312305

313306

314-
template <class T, Kind kind>
307+
template <class T>
315308
static void WeakCallback(const WeakCallbackData<T, ContextifyContext>& data) {
316309
ContextifyContext* context = data.GetParameter();
317-
if (kind == kSandbox)
318-
context->sandbox_.ClearWeak();
319-
else
320-
context->context_.ClearWeak();
321-
322-
if (--context->references_ == 0)
323-
delete context;
310+
context->context_.ClearWeak();
311+
delete context;
324312
}
325313

326314

@@ -342,26 +330,23 @@ class ContextifyContext {
342330
static void GlobalPropertyGetterCallback(
343331
Local<Name> property,
344332
const PropertyCallbackInfo<Value>& args) {
345-
Isolate* isolate = args.GetIsolate();
346-
347333
ContextifyContext* ctx =
348334
Unwrap<ContextifyContext>(args.Data().As<Object>());
349335

350336
// Stil initializing
351337
if (ctx->context_.IsEmpty())
352338
return;
353339

354-
Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);
355340
MaybeLocal<Value> maybe_rv =
356-
sandbox->GetRealNamedProperty(ctx->context(), property);
341+
ctx->sandbox()->GetRealNamedProperty(ctx->context(), property);
357342
if (maybe_rv.IsEmpty()) {
358343
maybe_rv =
359344
ctx->global_proxy()->GetRealNamedProperty(ctx->context(), property);
360345
}
361346

362347
Local<Value> rv;
363348
if (maybe_rv.ToLocal(&rv)) {
364-
if (rv == ctx->sandbox_)
349+
if (rv == ctx->sandbox())
365350
rv = ctx->global_proxy();
366351

367352
args.GetReturnValue().Set(rv);
@@ -373,34 +358,30 @@ class ContextifyContext {
373358
Local<Name> property,
374359
Local<Value> value,
375360
const PropertyCallbackInfo<Value>& args) {
376-
Isolate* isolate = args.GetIsolate();
377-
378361
ContextifyContext* ctx =
379362
Unwrap<ContextifyContext>(args.Data().As<Object>());
380363

381364
// Stil initializing
382365
if (ctx->context_.IsEmpty())
383366
return;
384367

385-
PersistentToLocal(isolate, ctx->sandbox_)->Set(property, value);
368+
ctx->sandbox()->Set(property, value);
386369
}
387370

388371

389372
static void GlobalPropertyQueryCallback(
390373
Local<Name> property,
391374
const PropertyCallbackInfo<Integer>& args) {
392-
Isolate* isolate = args.GetIsolate();
393-
394375
ContextifyContext* ctx =
395376
Unwrap<ContextifyContext>(args.Data().As<Object>());
396377

397378
// Stil initializing
398379
if (ctx->context_.IsEmpty())
399380
return;
400381

401-
Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);
402382
Maybe<PropertyAttribute> maybe_prop_attr =
403-
sandbox->GetRealNamedPropertyAttributes(ctx->context(), property);
383+
ctx->sandbox()->GetRealNamedPropertyAttributes(ctx->context(),
384+
property);
404385

405386
if (maybe_prop_attr.IsNothing()) {
406387
maybe_prop_attr =
@@ -418,18 +399,14 @@ class ContextifyContext {
418399
static void GlobalPropertyDeleterCallback(
419400
Local<Name> property,
420401
const PropertyCallbackInfo<Boolean>& args) {
421-
Isolate* isolate = args.GetIsolate();
422-
423402
ContextifyContext* ctx =
424403
Unwrap<ContextifyContext>(args.Data().As<Object>());
425404

426405
// Stil initializing
427406
if (ctx->context_.IsEmpty())
428407
return;
429408

430-
Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);
431-
432-
Maybe<bool> success = sandbox->Delete(ctx->context(), property);
409+
Maybe<bool> success = ctx->sandbox()->Delete(ctx->context(), property);
433410

434411
if (success.IsJust())
435412
args.GetReturnValue().Set(success.FromJust());
@@ -445,8 +422,7 @@ class ContextifyContext {
445422
if (ctx->context_.IsEmpty())
446423
return;
447424

448-
Local<Object> sandbox = PersistentToLocal(args.GetIsolate(), ctx->sandbox_);
449-
args.GetReturnValue().Set(sandbox->GetPropertyNames());
425+
args.GetReturnValue().Set(ctx->sandbox()->GetPropertyNames());
450426
}
451427
};
452428

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL