Skip to content

Commit

Permalink
Allow SetWeak() for non-object persistent handles. (#824)
Browse files Browse the repository at this point in the history 
The definition of the SetWeak() template method constrained the type to
types with a GetAlignedPointerFromInternalField() method.  In practice
that meant it only worked with T=v8::Object.

Lift that restriction so that e.g. T=v8::External now also works.  Note
that `SetWeak(WeakCallbackType::kInternalFields)` for types without the
aforementioned method will result in a run-time assertion.
  • Loading branch information
@bnoordhuis @kkoopa
bnoordhuis authored and kkoopa committed Nov 20, 2018
1 parent 801f129 commit e6ef6a4
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 23 deletions.
51 changes: 28 additions & 23 deletions nan_weak.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,20 +270,21 @@ inline void Persistent<T, M>::SetWeak(
, WeakCallbackInfo<P>::template invokeparameter<true>
, type);
} else {
v8::Local<T>* self = reinterpret_cast<v8::Local<T>*>(this);
assert((*self)->IsObject());
int count = (*self)->InternalFieldCount();
v8::Local<v8::Value>* self_v(reinterpret_cast<v8::Local<v8::Value>*>(this));
assert((*self_v)->IsObject());
v8::Local<v8::Object> self((*self_v).As<v8::Object>());
int count = self->InternalFieldCount();
void *internal_fields[kInternalFieldsInWeakCallback] = {0, 0};
for (int i = 0; i < count && i < kInternalFieldsInWeakCallback; i++) {
internal_fields[i] = (*self)->GetAlignedPointerFromInternalField(i);
internal_fields[i] = self->GetAlignedPointerFromInternalField(i);
}
wcbd = new WeakCallbackInfo<P>(
reinterpret_cast<Persistent<v8::Value>*>(this)
, callback
, 0
, internal_fields[0]
, internal_fields[1]);
(*self)->SetAlignedPointerInInternalField(0, wcbd);
self->SetAlignedPointerInInternalField(0, wcbd);
v8::PersistentBase<T>::SetWeak(
static_cast<WeakCallbackInfo<P>*>(0)
, WeakCallbackInfo<P>::template invoketwofield<true>
Expand All @@ -307,20 +308,21 @@ inline void Persistent<T, M>::SetWeak(
wcbd
, WeakCallbackInfo<P>::invokeparameter);
} else {
v8::Local<T>* self = reinterpret_cast<v8::Local<T>*>(this);
assert((*self)->IsObject());
int count = (*self)->InternalFieldCount();
v8::Local<v8::Value>* self_v(reinterpret_cast<v8::Local<v8::Value>*>(this));
assert((*self_v)->IsObject());
v8::Local<v8::Object> self((*self_v).As<v8::Object>());
int count = self->InternalFieldCount();
void *internal_fields[kInternalFieldsInWeakCallback] = {0, 0};
for (int i = 0; i < count && i < kInternalFieldsInWeakCallback; i++) {
internal_fields[i] = (*self)->GetAlignedPointerFromInternalField(i);
internal_fields[i] = self->GetAlignedPointerFromInternalField(i);
}
wcbd = new WeakCallbackInfo<P>(
reinterpret_cast<Persistent<v8::Value>*>(this)
, callback
, 0
, internal_fields[0]
, internal_fields[1]);
(*self)->SetAlignedPointerInInternalField(0, wcbd);
self->SetAlignedPointerInInternalField(0, wcbd);
v8::PersistentBase<T>::SetPhantom(
static_cast<WeakCallbackInfo<P>*>(0)
, WeakCallbackInfo<P>::invoketwofield
Expand All @@ -345,20 +347,21 @@ inline void Persistent<T, M>::SetWeak(
wcbd
, WeakCallbackInfo<P>::invokeparameter);
} else {
v8::Local<T>* self = reinterpret_cast<v8::Local<T>*>(this);
assert((*self)->IsObject());
int count = (*self)->InternalFieldCount();
v8::Local<v8::Value>* self_v(reinterpret_cast<v8::Local<v8::Value>*>(this));
assert((*self_v)->IsObject());
v8::Local<v8::Object> self((*self_v).As<v8::Object>());
int count = self->InternalFieldCount();
void *internal_fields[kInternalFieldsInWeakCallback] = {0, 0};
for (int i = 0; i < count && i < kInternalFieldsInWeakCallback; i++) {
internal_fields[i] = (*self)->GetAlignedPointerFromInternalField(i);
internal_fields[i] = self->GetAlignedPointerFromInternalField(i);
}
wcbd = new WeakCallbackInfo<P>(
reinterpret_cast<Persistent<v8::Value>*>(this)
, callback
, 0
, internal_fields[0]
, internal_fields[1]);
(*self)->SetAlignedPointerInInternalField(0, wcbd);
self->SetAlignedPointerInInternalField(0, wcbd);
v8::PersistentBase<T>::SetPhantom(
WeakCallbackInfo<P>::invoketwofield
, 0
Expand All @@ -380,12 +383,13 @@ inline void Persistent<T, M>::SetWeak(
, parameter);
v8::PersistentBase<T>::SetWeak(wcbd, WeakCallbackInfo<P>::invoke);
} else {
v8::Local<T>* self = reinterpret_cast<v8::Local<T>*>(this);
assert((*self)->IsObject());
int count = (*self)->InternalFieldCount();
v8::Local<v8::Value>* self_v(reinterpret_cast<v8::Local<v8::Value>*>(this));
assert((*self_v)->IsObject());
v8::Local<v8::Object> self((*self_v).As<v8::Object>());
int count = self->InternalFieldCount();
void *internal_fields[kInternalFieldsInWeakCallback] = {0, 0};
for (int i = 0; i < count && i < kInternalFieldsInWeakCallback; i++) {
internal_fields[i] = (*self)->GetAlignedPointerFromInternalField(i);
internal_fields[i] = self->GetAlignedPointerFromInternalField(i);
}
wcbd = new WeakCallbackInfo<P>(
reinterpret_cast<Persistent<v8::Value>*>(this)
Expand All @@ -411,12 +415,13 @@ inline void PersistentBase<T>::SetWeak(
, parameter);
persistent.MakeWeak(wcbd, WeakCallbackInfo<P>::invoke);
} else {
v8::Local<T>* self = reinterpret_cast<v8::Local<T>*>(this);
assert((*self)->IsObject());
int count = (*self)->InternalFieldCount();
v8::Local<v8::Value>* self_v(reinterpret_cast<v8::Local<v8::Value>*>(this));
assert((*self_v)->IsObject());
v8::Local<v8::Object> self((*self_v).As<v8::Object>());
int count = self->InternalFieldCount();
void *internal_fields[kInternalFieldsInWeakCallback] = {0, 0};
for (int i = 0; i < count && i < kInternalFieldsInWeakCallback; i++) {
internal_fields[i] = (*self)->GetPointerFromInternalField(i);
internal_fields[i] = self->GetPointerFromInternalField(i);
}
wcbd = new WeakCallbackInfo<P>(
reinterpret_cast<Persistent<v8::Value>*>(this)
Expand Down
12 changes: 12 additions & 0 deletions test/cpp/weak.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,23 @@ NAN_METHOD(Hustle) {
info.GetReturnValue().Set(wrap(To<v8::Function>(info[0]).ToLocalChecked()));
}

inline void WeakExternalCallback(const WeakCallbackInfo<void>&) {}

NAN_METHOD(WeakExternal) {
void* baton = &baton; // Actual value doesn't really matter.
Persistent<v8::External> external(New<v8::External>(baton));
external.SetWeak(baton, WeakExternalCallback, WeakCallbackType::kParameter);
}

NAN_MODULE_INIT(Init) {
Set(target
, New<v8::String>("hustle").ToLocalChecked()
, New<v8::FunctionTemplate>(Hustle)->GetFunction()
);
Set(target
, New<v8::String>("weakExternal").ToLocalChecked()
, New<v8::FunctionTemplate>(WeakExternal)->GetFunction()
);
}

NODE_MODULE(weak, Init)
10 changes: 10 additions & 0 deletions test/js/weak-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,13 @@ test('weak', function (t) {
}
}, 100);
});

test('weak external', function (t) {
t.plan(2);

var weak = bindings;
t.type(weak.weakExternal, 'function');

weak.weakExternal();
t.ok(true); // All good if the previous line didn't crash.
});

0 comments on commit e6ef6a4

Please sign in to comment.