Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Commit

Permalink
chakrashim: Fix Promise::Resolver
Browse files Browse the repository at this point in the history 
The promisify function calls into native code to create a new
Promise::Resolver object and returns it back to script. In script it's
expected to be a promise object, but in our case it wasn't.

The solution is to hide the resolver data in an external object and
attach it to the promise object.

PR-URL: #470
Reviewed-By: Jimmy Thomson <[email protected]>
Reviewed-By: Hitesh Kanwathirtha <[email protected]>
  • Loading branch information
@kfarnung
kfarnung committed Mar 7, 2018
1 parent c850835 commit ae4cea5
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 140 deletions.
1 change: 1 addition & 0 deletions deps/chakrashim/include/v8.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,7 @@ class Persistent : public PersistentBase<T> {
friend class Utils;
template <class F> friend class Local;
template <class F> friend class ReturnValue;
friend class PromiseResolverData;

V8_INLINE Persistent(T* that)
: PersistentBase<T>(PersistentBase<T>::New(nullptr, that)) { }
Expand Down
18 changes: 8 additions & 10 deletions deps/chakrashim/src/jsrtutils.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,12 @@ JsErrorCode GetPropertyNames(JsValueRef object,
object, result);
}

JsPropertyIdRef GetExternalPropertyId() {
IsolateShim* iso = IsolateShim::GetCurrent();
return iso->GetCachedSymbolPropertyIdRef(
CachedSymbolPropertyIdRef::__external__);
}

JsErrorCode AddExternalData(JsValueRef ref,
JsPropertyIdRef externalDataPropertyId,
void *data,
Expand All @@ -413,11 +419,7 @@ JsErrorCode AddExternalData(JsValueRef ref,
JsErrorCode AddExternalData(JsValueRef ref,
void *data,
JsFinalizeCallback onObjectFinalize) {
IsolateShim* iso = IsolateShim::GetCurrent();
JsPropertyIdRef propId = iso->GetCachedSymbolPropertyIdRef(
CachedSymbolPropertyIdRef::__external__);

return AddExternalData(ref, propId, data, onObjectFinalize);
return AddExternalData(ref, GetExternalPropertyId(), data, onObjectFinalize);
}

JsErrorCode GetExternalData(JsValueRef ref,
Expand All @@ -437,11 +439,7 @@ JsErrorCode GetExternalData(JsValueRef ref,

JsErrorCode GetExternalData(JsValueRef ref,
void **data) {
IsolateShim* iso = IsolateShim::GetCurrent();
JsPropertyIdRef propId = iso->GetCachedSymbolPropertyIdRef(
CachedSymbolPropertyIdRef::__external__);

return GetExternalData(ref, propId, data);
return GetExternalData(ref, GetExternalPropertyId(), data);
}

JsErrorCode CreateFunctionWithExternalData(
Expand Down
2 changes: 2 additions & 0 deletions deps/chakrashim/src/jsrtutils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@ JsErrorCode CloneObject(JsValueRef source,
JsErrorCode GetPropertyNames(JsValueRef object,
JsValueRef *namesArray);

JsPropertyIdRef GetExternalPropertyId();

JsErrorCode AddExternalData(JsValueRef ref,
JsPropertyIdRef externalDataPropertyId,
void *data,
Expand Down
12 changes: 12 additions & 0 deletions deps/chakrashim/src/v8chakra.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ enum class ExternalDataTypes {
ObjectData,
FunctionTemplateData,
FunctionCallbackData,
PromiseResolverData,
};

// Base class for external object data
Expand Down Expand Up @@ -75,6 +76,17 @@ class ExternalData {
static bool TryGet(JsValueRef ref, T** data) {
return GetExternalData(ref, data) == JsNoError && *data != nullptr;
}

template <class T>
static bool TryGetFromProperty(JsValueRef ref, JsPropertyIdRef propertyId,
T** data) {
JsValueRef propertyValue = nullptr;
if (JsGetProperty(ref, propertyId, &propertyValue) != JsNoError) {
return false;
}

return T::TryGet(propertyValue, data);
}
};

struct SetterGetterInterceptor {
Expand Down
250 changes: 120 additions & 130 deletions deps/chakrashim/src/v8resolver.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,164 +21,154 @@
#include "v8chakra.h"
#include "jsrtutils.h"

namespace v8 {
using Resolver = Promise::Resolver;
// CHAKRA-TODO: Unimplemented completely
MaybeLocal<Resolver> Resolver::New(Local<Context> context) {
JsValueRef resolver;
if (JsCreateObject(&resolver) != JsNoError) {
return Local<Resolver>();
}

JsValueRef promise, resolve, reject;
if (JsCreatePromise(&promise, &resolve, &reject) != JsNoError) {
return Local<Resolver>();
}
#include <array>

if (jsrt::SetProperty(
resolver,
jsrt::CachedPropertyIdRef::promise,
promise) != JsNoError) {
return Local<Resolver>();
}

if (jsrt::SetProperty(
resolver,
jsrt::CachedPropertyIdRef::resolve,
resolve) != JsNoError) {
return Local<Resolver>();
}
namespace v8 {

if (jsrt::SetProperty(
resolver,
jsrt::CachedPropertyIdRef::reject,
reject) != JsNoError) {
return Local<Resolver>();
}
class PromiseResolverData : public ExternalData {
public:
static const ExternalDataTypes ExternalDataType =
ExternalDataTypes::PromiseResolverData;

private:
Persistent<Value> resolve;
Persistent<Value> reject;

public:
PromiseResolverData(Local<Value> resolve,
Local<Value> reject)
: ExternalData(ExternalDataType),
resolve(nullptr, resolve),
reject(nullptr, reject) {}

~PromiseResolverData() {
resolve.Reset();
reject.Reset();
}

JsValueRef state;
jsrt::UintToValue(0, &state);
if (jsrt::SetProperty(
promise,
jsrt::CachedPropertyIdRef::value,
state) != JsNoError) {
return Local<Resolver>();
static void CHAKRA_CALLBACK FinalizeCallback(void *data) {
if (data != nullptr) {
PromiseResolverData* promiseResolverData =
reinterpret_cast<PromiseResolverData*>(data);
delete promiseResolverData;
}
}

return Local<Resolver>::New(static_cast<Resolver*>(resolver));
JsErrorCode Resolve(Local<Value> value) {
JsValueRef result = nullptr;
std::array<JsValueRef, 2> args = { jsrt::GetUndefined(), *value };
return JsCallFunction(*resolve, args.data(), args.size(), &result);
}

Local<Resolver> Resolver::New(Isolate* isolate) {
return New(isolate->GetCurrentContext()).ToLocalChecked();
JsErrorCode Reject(Local<Value> value) {
JsValueRef result = nullptr;
std::array<JsValueRef, 2> args = { jsrt::GetUndefined(), *value };
return JsCallFunction(*reject, args.data(), args.size(), &result);
}
};

Local<Promise> Resolver::GetPromise() {
JsValueRef promise;
if (jsrt::GetProperty(
this,
jsrt::CachedPropertyIdRef::promise,
&promise) != JsNoError) {
return Local<Promise>();
}
return Local<Promise>::New(static_cast<Promise*>(promise));
MaybeLocal<Promise::Resolver> Promise::Resolver::New(Local<Context> context) {
JsValueRef promise, resolve, reject;
if (JsCreatePromise(&promise, &resolve, &reject) != JsNoError) {
return Local<Promise::Resolver>();
}

Resolver* Resolver::Cast(Value* obj) {
return static_cast<Resolver*>(obj);
PromiseResolverData* data = new PromiseResolverData(resolve, reject);
if (jsrt::AddExternalData(
promise, data, PromiseResolverData::FinalizeCallback) != JsNoError) {
delete data;
return Local<Promise::Resolver>();
}

Maybe<bool> Resolver::Resolve(Local<Context> context, Local<Value> value) {
JsValueRef resolve;
if (jsrt::GetProperty(
this,
jsrt::CachedPropertyIdRef::resolve,
&resolve) != JsNoError) {
return Nothing<bool>();
}
JsValueRef state;
jsrt::UintToValue(0, &state);
if (jsrt::SetProperty(
promise,
jsrt::CachedPropertyIdRef::state,
state) != JsNoError) {
return Local<Promise::Resolver>();
}

JsValueRef promise;
if (jsrt::GetProperty(
this,
jsrt::CachedPropertyIdRef::promise,
&promise) != JsNoError) {
return Nothing<bool>();
}
return Local<Promise::Resolver>::New(promise);
}

if (jsrt::SetProperty(
promise,
jsrt::CachedPropertyIdRef::value,
*value) != JsNoError) {
return Nothing<bool>();
}
Local<Promise::Resolver> Promise::Resolver::New(Isolate* isolate) {
return New(isolate->GetCurrentContext()).ToLocalChecked();
}

JsValueRef state;
jsrt::UintToValue(1, &state);
if (jsrt::SetProperty(
promise,
jsrt::CachedPropertyIdRef::value,
state) != JsNoError) {
return Nothing<bool>();
}
Local<Promise> Promise::Resolver::GetPromise() {
return Local<Promise>::New(static_cast<JsValueRef>(this));
}

JsValueRef result;
JsValueRef args[2];
args[0] = this; // ? What is the "this" of the resolver here?
args[1] = reinterpret_cast<JsValueRef>(*value);
Promise::Resolver* Promise::Resolver::Cast(Value* obj) {
return static_cast<Promise::Resolver*>(obj);
}

if (JsCallFunction(resolve, args, 2, &result) != JsNoError) {
return Nothing<bool>();
}
Maybe<bool> Promise::Resolver::Resolve(Local<Context> context,
Local<Value> value) {
PromiseResolverData* data = nullptr;
if (!ExternalData::TryGetFromProperty(this, jsrt::GetExternalPropertyId(),
&data)) {
return Nothing<bool>();
}

return Just(true);
if (jsrt::SetProperty(
this,
jsrt::CachedPropertyIdRef::value,
*value) != JsNoError) {
return Nothing<bool>();
}

void Resolver::Resolve(Local<Value> value) {
Local<Context> context;
Resolve(context, value);
JsValueRef state;
jsrt::UintToValue(1, &state);
if (jsrt::SetProperty(
this,
jsrt::CachedPropertyIdRef::state,
state) != JsNoError) {
return Nothing<bool>();
}

Maybe<bool> Resolver::Reject(Local<Context> context, Local<Value> value) {
JsValueRef reject;
if (jsrt::GetProperty(
this,
jsrt::CachedPropertyIdRef::reject,
&reject) != JsNoError) {
return Nothing<bool>();
}
if (data->Resolve(value) != JsNoError) {
return Nothing<bool>();
}

JsValueRef promise;
if (jsrt::GetProperty(
this,
jsrt::CachedPropertyIdRef::promise,
&promise) != JsNoError) {
return Nothing<bool>();
}
return Just(true);
}

if (jsrt::SetProperty(
promise,
jsrt::CachedPropertyIdRef::value,
*value) != JsNoError) {
return Nothing<bool>();
}
void Promise::Resolver::Resolve(Local<Value> value) {
Local<Context> context;
Resolve(context, value);
}

JsValueRef state;
jsrt::UintToValue(2, &state);
if (jsrt::SetProperty(
promise,
jsrt::CachedPropertyIdRef::value,
state) != JsNoError) {
return Nothing<bool>();
}
Maybe<bool> Promise::Resolver::Reject(Local<Context> context,
Local<Value> value) {
PromiseResolverData* data = nullptr;
if (!ExternalData::TryGetFromProperty(this, jsrt::GetExternalPropertyId(),
&data)) {
return Nothing<bool>();
}

JsValueRef result;
JsValueRef args[2];
args[0] = this; // ? What is the "this" of the resolver here?
args[1] = reinterpret_cast<JsValueRef>(*value);
if (jsrt::SetProperty(
this,
jsrt::CachedPropertyIdRef::value,
*value) != JsNoError) {
return Nothing<bool>();
}

if (JsCallFunction(reject, args, 2, &result) != JsNoError) {
return Nothing<bool>();
}
JsValueRef state;
jsrt::UintToValue(2, &state);
if (jsrt::SetProperty(
this,
jsrt::CachedPropertyIdRef::state,
state) != JsNoError) {
return Nothing<bool>();
}

return Just(true);
if (data->Reject(value) != JsNoError) {
return Nothing<bool>();
}

return Just(true);
}

} // namespace v8

0 comments on commit ae4cea5

Please sign in to comment.