From e87ceb2b427a9b51a55af5898c2ef24b4cc08af2 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 15 Mar 2014 01:12:53 +0100 Subject: [PATCH] src: fix up smalloc weak persistent usage Fix a regression that was introduced in commit ce04c726 after the upgrade to V8 3.24. The new weak persistent handle API no longer gives you the original persistent but still requires that you clear it inside your weak callback. Rearrange the code in src/smalloc.cc to keep track of the persistent handle with the least amount of pain and try hard to share as much code as possible between the 'just free it' and 'invoke my callback' versions of the smalloc API. Fixes #7309. --- src/smalloc.cc | 233 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 151 insertions(+), 82 deletions(-) diff --git a/src/smalloc.cc b/src/smalloc.cc index a5c5015a98..1e9cc3621a 100644 --- a/src/smalloc.cc +++ b/src/smalloc.cc @@ -54,17 +54,148 @@ using v8::WeakCallbackData; using v8::kExternalUnsignedByteArray; -struct CallbackInfo { - void* hint; - FreeCallback cb; - Persistent p_obj; +template +class CallbackInfo { + public: + static inline CallbackInfo* New(Isolate* isolate, + Handle object, + Payload payload); + inline void Dispose(); + inline Persistent* persistent(); + inline Payload* payload(); + private: + static void WeakCallback(const WeakCallbackData&); + inline CallbackInfo(Isolate* isolate, + Handle object, + Payload payload); + ~CallbackInfo(); + Persistent persistent_; + Payload payload_; }; -void TargetCallback(const WeakCallbackData& data); -void TargetFreeCallback(Isolate* isolate, - Persistent* target, - CallbackInfo* cb_info); -void TargetFreeCallback(const WeakCallbackData& data); + +class Free { + public: + explicit Free(char* data); + void WeakCallback(Isolate* isolate, + Local object, + CallbackInfo* info); + private: + char* const data_; +}; + + +class Dispose { + public: + typedef void (*Callback)(char* data, void* hint); + Dispose(Callback callback, void* hint); + void WeakCallback(Isolate* isolate, + Local object, + CallbackInfo* info); + private: + Callback const callback_; + void* const hint_; +}; + + +template +CallbackInfo* CallbackInfo::New(Isolate* isolate, + Handle object, + Payload payload) { + return new CallbackInfo(isolate, object, payload); +} + + +template +void CallbackInfo::Dispose() { + delete this; +} + + +template +Persistent* CallbackInfo::persistent() { + return &persistent_; +} + + +template +Payload* CallbackInfo::payload() { + return &payload_; +} + + +template +CallbackInfo::CallbackInfo(Isolate* isolate, + Handle object, + Payload payload) + : persistent_(isolate, object), + payload_(payload) { + persistent_.SetWeak(this, WeakCallback); + persistent_.SetWrapperClassId(ALLOC_ID); + persistent_.MarkIndependent(); +} + + +template +CallbackInfo::~CallbackInfo() { + persistent_.Reset(); +} + + +template +void CallbackInfo::WeakCallback( + const WeakCallbackData& data) { + CallbackInfo* info = data.GetParameter(); + info->payload()->WeakCallback(data.GetIsolate(), data.GetValue(), info); +} + + +Free::Free(char* data) : data_(data) { +} + + +void Free::WeakCallback(Isolate* isolate, + Local object, + CallbackInfo* info) { + free(data_); + size_t length = object->GetIndexedPropertiesExternalArrayDataLength(); + enum ExternalArrayType array_type = + object->GetIndexedPropertiesExternalArrayDataType(); + size_t array_size = ExternalArraySize(array_type); + CHECK_GT(array_size, 0); + CHECK_GE(array_size * length, length); // Overflow check. + length *= array_size; + int64_t change_in_bytes = -static_cast(length); + isolate->AdjustAmountOfExternalAllocatedMemory(change_in_bytes); + info->Dispose(); +} + + +Dispose::Dispose(Callback callback, void* hint) + : callback_(callback), + hint_(hint) { +} + + +void Dispose::WeakCallback(Isolate* isolate, + Local object, + CallbackInfo* info) { + char* data = + static_cast(object->GetIndexedPropertiesExternalArrayData()); + size_t len = object->GetIndexedPropertiesExternalArrayDataLength(); + enum ExternalArrayType array_type = + object->GetIndexedPropertiesExternalArrayDataType(); + size_t array_size = ExternalArraySize(array_type); + CHECK_GT(array_size, 0); + if (array_size > 1) { + CHECK_GT(len * array_size, len); // Overflow check. + len *= array_size; + } + int64_t change_in_bytes = -static_cast(len + sizeof(*info)); + isolate->AdjustAmountOfExternalAllocatedMemory(change_in_bytes); + callback_(data, hint_); + info->Dispose(); +} // return size of external array type, or 0 if unrecognized @@ -268,36 +399,10 @@ void Alloc(Environment* env, size_t length, enum ExternalArrayType type) { assert(!obj->HasIndexedPropertiesInExternalArrayData()); - Persistent p_obj(env->isolate(), obj); env->isolate()->AdjustAmountOfExternalAllocatedMemory(length); - p_obj.SetWeak(data, TargetCallback); - p_obj.MarkIndependent(); - p_obj.SetWrapperClassId(ALLOC_ID); size_t size = length / ExternalArraySize(type); obj->SetIndexedPropertiesToExternalArrayData(data, type, size); -} - - -void TargetCallback(const WeakCallbackData& data) { - HandleScope handle_scope(data.GetIsolate()); - char* info = data.GetParameter(); - - Local obj = data.GetValue(); - size_t len = obj->GetIndexedPropertiesExternalArrayDataLength(); - enum ExternalArrayType array_type = - obj->GetIndexedPropertiesExternalArrayDataType(); - size_t array_size = ExternalArraySize(array_type); - assert(array_size > 0); - assert(array_size * len >= len); - len *= array_size; - if (info != NULL && len > 0) { - int64_t change_in_bytes = -static_cast(len); - data.GetIsolate()->AdjustAmountOfExternalAllocatedMemory(change_in_bytes); - free(info); - } - - // XXX: Persistent is no longer passed here - // persistent.Reset(); + CallbackInfo::New(env->isolate(), obj, Free(data)); } @@ -315,8 +420,10 @@ void AllocDispose(Environment* env, Handle obj) { Local ext_v = obj->GetHiddenValue(env->smalloc_p_string()); if (ext_v->IsExternal()) { Local ext = ext_v.As(); - CallbackInfo* cb_info = static_cast(ext->Value()); - TargetFreeCallback(env->isolate(), &cb_info->p_obj, cb_info); + CallbackInfo* info = static_cast*>(ext->Value()); + Local object = PersistentToLocal(env->isolate(), + *info->persistent()); + info->payload()->WeakCallback(env->isolate(), object, info); return; } } @@ -373,56 +480,18 @@ void Alloc(Environment* env, void* hint, enum ExternalArrayType type) { assert(!obj->HasIndexedPropertiesInExternalArrayData()); - - HandleScope handle_scope(env->isolate()); + Isolate* isolate = env->isolate(); + HandleScope handle_scope(isolate); env->set_using_smalloc_alloc_cb(true); - - CallbackInfo* cb_info = new CallbackInfo; - cb_info->cb = fn; - cb_info->hint = hint; - cb_info->p_obj.Reset(env->isolate(), obj); - obj->SetHiddenValue(env->smalloc_p_string(), - External::New(env->isolate(), cb_info)); - - env->isolate()->AdjustAmountOfExternalAllocatedMemory(length + - sizeof(*cb_info)); - cb_info->p_obj.SetWeak(cb_info, TargetFreeCallback); - cb_info->p_obj.MarkIndependent(); - cb_info->p_obj.SetWrapperClassId(ALLOC_ID); + CallbackInfo* info = + CallbackInfo::New(isolate, obj, Dispose(fn, hint)); + obj->SetHiddenValue(env->smalloc_p_string(), External::New(isolate, info)); + isolate->AdjustAmountOfExternalAllocatedMemory(length + sizeof(*info)); size_t size = length / ExternalArraySize(type); obj->SetIndexedPropertiesToExternalArrayData(data, type, size); } -void TargetFreeCallback(Isolate* isolate, - Persistent* target, - CallbackInfo* cb_info) { - HandleScope handle_scope(isolate); - Local obj = PersistentToLocal(isolate, *target); - char* data = static_cast(obj->GetIndexedPropertiesExternalArrayData()); - size_t len = obj->GetIndexedPropertiesExternalArrayDataLength(); - enum ExternalArrayType array_type = - obj->GetIndexedPropertiesExternalArrayDataType(); - size_t array_size = ExternalArraySize(array_type); - assert(array_size > 0); - if (array_size > 1) { - assert(len * array_size > len); - len *= array_size; - } - int64_t change_in_bytes = -static_cast(len + sizeof(*cb_info)); - isolate->AdjustAmountOfExternalAllocatedMemory(change_in_bytes); - cb_info->p_obj.Reset(); - cb_info->cb(data, cb_info->hint); - delete cb_info; -} - - -void TargetFreeCallback(const WeakCallbackData& data) { - CallbackInfo* cb_info = data.GetParameter(); - TargetFreeCallback(data.GetIsolate(), &cb_info->p_obj, cb_info); -} - - void HasExternalData(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args.GetIsolate()); args.GetReturnValue().Set(args[0]->IsObject() &&