Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement HostPromiseRejectionTracker (#2530) #4608

Merged
merged 1 commit into from
Feb 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions bin/ChakraCore/ChakraCore.def
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,5 @@ JsLessThan
JsLessThanOrEqual

JsCreateEnhancedFunction

JsSetHostPromiseRejectionTracker
1 change: 1 addition & 0 deletions bin/ch/ChakraRtInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ bool ChakraRTInterface::LoadChakraDll(ArgInfo* argInfo, HINSTANCE *outLibrary)
m_jsApiHooks.pfJsrtGetValueType = (JsAPIHooks::JsrtGetValueType)GetChakraCoreSymbol(library, "JsGetValueType");
m_jsApiHooks.pfJsrtSetIndexedProperty = (JsAPIHooks::JsrtSetIndexedPropertyPtr)GetChakraCoreSymbol(library, "JsSetIndexedProperty");
m_jsApiHooks.pfJsrtSetPromiseContinuationCallback = (JsAPIHooks::JsrtSetPromiseContinuationCallbackPtr)GetChakraCoreSymbol(library, "JsSetPromiseContinuationCallback");
m_jsApiHooks.pfJsrtSetHostPromiseRejectionTracker = (JsAPIHooks::JsrtSetHostPromiseRejectionTrackerPtr)GetChakraCoreSymbol(library, "JsSetHostPromiseRejectionTracker");
m_jsApiHooks.pfJsrtGetContextOfObject = (JsAPIHooks::JsrtGetContextOfObject)GetChakraCoreSymbol(library, "JsGetContextOfObject");
m_jsApiHooks.pfJsrtInitializeModuleRecord = (JsAPIHooks::JsInitializeModuleRecordPtr)GetChakraCoreSymbol(library, "JsInitializeModuleRecord");
m_jsApiHooks.pfJsrtParseModuleSource = (JsAPIHooks::JsParseModuleSourcePtr)GetChakraCoreSymbol(library, "JsParseModuleSource");
Expand Down
3 changes: 3 additions & 0 deletions bin/ch/ChakraRtInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ struct JsAPIHooks
typedef JsErrorCode (WINAPI *JsrtGetValueType)(JsValueRef value, JsValueType *type);
typedef JsErrorCode (WINAPI *JsrtSetIndexedPropertyPtr)(JsValueRef object, JsValueRef index, JsValueRef value);
typedef JsErrorCode (WINAPI *JsrtSetPromiseContinuationCallbackPtr)(JsPromiseContinuationCallback callback, void *callbackState);
typedef JsErrorCode (WINAPI *JsrtSetHostPromiseRejectionTrackerPtr)(JsHostPromiseRejectionTrackerCallback callback, void *callbackState);
typedef JsErrorCode (WINAPI *JsrtGetContextOfObject)(JsValueRef object, JsContextRef *callbackState);

typedef JsErrorCode(WINAPI *JsrtDiagStartDebugging)(JsRuntimeHandle runtimeHandle, JsDiagDebugEventCallback debugEventCallback, void* callbackState);
Expand Down Expand Up @@ -152,6 +153,7 @@ struct JsAPIHooks
JsrtGetValueType pfJsrtGetValueType;
JsrtSetIndexedPropertyPtr pfJsrtSetIndexedProperty;
JsrtSetPromiseContinuationCallbackPtr pfJsrtSetPromiseContinuationCallback;
JsrtSetHostPromiseRejectionTrackerPtr pfJsrtSetHostPromiseRejectionTracker;
JsrtGetContextOfObject pfJsrtGetContextOfObject;
JsrtDiagStartDebugging pfJsrtDiagStartDebugging;
JsrtDiagStopDebugging pfJsrtDiagStopDebugging;
Expand Down Expand Up @@ -356,6 +358,7 @@ class ChakraRTInterface
static JsErrorCode WINAPI JsGetValueType(JsValueRef value, JsValueType *type) { return HOOK_JS_API(GetValueType(value, type)); }
static JsErrorCode WINAPI JsSetIndexedProperty(JsValueRef object, JsValueRef index, JsValueRef value) { return HOOK_JS_API(SetIndexedProperty(object, index, value)); }
static JsErrorCode WINAPI JsSetPromiseContinuationCallback(JsPromiseContinuationCallback callback, void *callbackState) { return HOOK_JS_API(SetPromiseContinuationCallback(callback, callbackState)); }
static JsErrorCode WINAPI JsSetHostPromiseRejectionTracker(JsHostPromiseRejectionTrackerCallback callback, void *callbackState) { return HOOK_JS_API(SetHostPromiseRejectionTracker(callback, callbackState)); }
static JsErrorCode WINAPI JsGetContextOfObject(JsValueRef object, JsContextRef* context) { return HOOK_JS_API(GetContextOfObject(object, context)); }
static JsErrorCode WINAPI JsDiagStartDebugging(JsRuntimeHandle runtimeHandle, JsDiagDebugEventCallback debugEventCallback, void* callbackState) { return HOOK_JS_API(DiagStartDebugging(runtimeHandle, debugEventCallback, callbackState)); }
static JsErrorCode WINAPI JsDiagStopDebugging(JsRuntimeHandle runtimeHandle, void** callbackState) { return HOOK_JS_API(DiagStopDebugging(runtimeHandle, callbackState)); }
Expand Down
1 change: 1 addition & 0 deletions bin/ch/HostConfigFlagsList.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ FLAG(bool, IgnoreScriptErrorCode, "Don't return error code on script e
FLAG(bool, MuteHostErrorMsg, "Mute host error output, e.g. module load failures", false)
FLAG(bool, TraceHostCallback, "Output traces for host callbacks", false)
FLAG(bool, Test262, "load Test262 harness", false)
FLAG(bool, TrackRejectedPromises, "Enable tracking of unhandled promise rejections", false)
#undef FLAG
#endif
29 changes: 29 additions & 0 deletions bin/ch/WScriptJsrt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1871,3 +1871,32 @@ void WScriptJsrt::PromiseContinuationCallback(JsValueRef task, void *callbackSta
WScriptJsrt::CallbackMessage *msg = new WScriptJsrt::CallbackMessage(0, task);
messageQueue->InsertSorted(msg);
}

void WScriptJsrt::PromiseRejectionTrackerCallback(JsValueRef promise, JsValueRef reason, bool handled, void *callbackState)
{
Assert(promise != JS_INVALID_REFERENCE);
Assert(reason != JS_INVALID_REFERENCE);
JsValueRef strValue;
JsErrorCode error = ChakraRTInterface::JsConvertValueToString(reason, &strValue);

if (!handled)
{
wprintf(_u("Uncaught promise rejection\n"));
}
else
{
wprintf(_u("Promise rejection handled\n"));
}

if (error == JsNoError)
{
AutoString str(strValue);
if (str.GetError() == JsNoError)
{
wprintf(_u("%ls\n"), str.GetWideString());
}
}

fflush(stdout);
}

1 change: 1 addition & 0 deletions bin/ch/WScriptJsrt.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class WScriptJsrt
static JsErrorCode NotifyModuleReadyCallback(_In_opt_ JsModuleRecord referencingModule, _In_opt_ JsValueRef exceptionVar);
static JsErrorCode InitializeModuleCallbacks();
static void CALLBACK PromiseContinuationCallback(JsValueRef task, void *callbackState);
static void CALLBACK PromiseRejectionTrackerCallback(JsValueRef promise, JsValueRef reason, bool handled, void *callbackState);

static LPCWSTR ConvertErrorCodeToMessage(JsErrorCode errorCode)
{
Expand Down
5 changes: 5 additions & 0 deletions bin/ch/ch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,11 @@ HRESULT ExecuteTest(const char* fileName)
IfFailGo(E_FAIL);
}

if (HostConfigFlags::flags.TrackRejectedPromises)
{
ChakraRTInterface::JsSetHostPromiseRejectionTracker(WScriptJsrt::PromiseRejectionTrackerCallback, nullptr);
}

len = strlen(fullPath);
if (HostConfigFlags::flags.GenerateLibraryByteCodeHeaderIsEnabled)
{
Expand Down
43 changes: 43 additions & 0 deletions lib/Jsrt/ChakraCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,27 @@ typedef struct JsNativeFunctionInfo
/// <returns>The result of the call, if any.</returns>
typedef _Ret_maybenull_ JsValueRef(CHAKRA_CALLBACK * JsEnhancedNativeFunction)(_In_ JsValueRef callee, _In_ JsValueRef *arguments, _In_ unsigned short argumentCount, _In_ JsNativeFunctionInfo *info, _In_opt_ void *callbackState);

/// <summary>
/// A Promise Rejection Tracker callback.
/// </summary>
/// <remarks>
/// The host can specify a promise rejection tracker callback in <c>JsSetHostPromiseRejectionTracker</c>.
/// If a promise is rejected with no reactions or a reaction is added to a promise that was rejected
/// before it had reactions by default nothing is done.
/// A Promise Rejection Tracker callback may be set - which will then be called when this occurs.
/// Note - per draft ECMASpec 2018 25.4.1.9 this function should not set or return an exception
/// Note also the promise and reason parameters may be garbage collected after this function is called
/// if you wish to make further use of them you will need to use JsAddRef to preserve them
/// However if you use JsAddRef you must also call JsRelease and not hold unto them after
/// a handled notification (both per spec and to avoid memory leaks)
/// </remarks>
/// <param name="promise">The promise object, represented as a JsValueRef.</param>
/// <param name="reason">The value/cause of the rejection, represented as a JsValueRef.</param>
/// <param name="handled">Boolean - false for promiseRejected: i.e. if the promise has just been rejected with no handler,
/// true for promiseHandled: i.e. if it was rejected before without a handler and is now being handled.</param>
/// <param name="callbackState">The state passed to <c>JsSetHostPromiseRejectionTracker</c>.</param>
typedef void (CHAKRA_CALLBACK *JsHostPromiseRejectionTrackerCallback)(_In_ JsValueRef promise, _In_ JsValueRef reason, _In_ bool handled, _In_opt_ void *callbackState);

/// <summary>
/// Creates a new enhanced JavaScript function.
/// </summary>
Expand Down Expand Up @@ -993,5 +1014,27 @@ CHAKRA_API
_In_ JsValueRef object,
_In_ JsValueRef key,
_Out_ bool *hasOwnProperty);

/// <summary>
/// Sets whether any action should be taken when a promise is rejected with no reactions
/// or a reaction is added to a promise that was rejected before it had reactions.
/// By default in either of these cases nothing occurs.
/// This function allows you to specify if something should occur and provide a callback
/// to implement whatever should occur.
/// </summary>
/// <remarks>
/// Requires an active script context.
/// </remarks>
/// <param name="promiseRejectionTrackerCallback">The callback function being set.</param>
/// <param name="callbackState">
/// User provided state that will be passed back to the callback.
/// </param>
/// <returns>
/// The code <c>JsNoError</c> if the operation succeeded, a failure code otherwise.
/// </returns>
CHAKRA_API
JsSetHostPromiseRejectionTracker(
_In_ JsHostPromiseRejectionTrackerCallback promiseRejectionTrackerCallback,
_In_opt_ void *callbackState);
#endif // _CHAKRACOREBUILD
#endif // _CHAKRACORE_H_
9 changes: 9 additions & 0 deletions lib/Jsrt/Jsrt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5334,4 +5334,13 @@ CHAKRA_API JsGetDataViewInfo(
END_JSRT_NO_EXCEPTION
}

CHAKRA_API JsSetHostPromiseRejectionTracker(_In_ JsHostPromiseRejectionTrackerCallback promiseRejectionTrackerCallback, _In_opt_ void *callbackState)
{
return ContextAPINoScriptWrapper_NoRecord([&](Js::ScriptContext *scriptContext) -> JsErrorCode {
scriptContext->GetLibrary()->SetNativeHostPromiseRejectionTrackerCallback((Js::JavascriptLibrary::HostPromiseRejectionTrackerCallback) promiseRejectionTrackerCallback, callbackState);
return JsNoError;
},
/*allowInObjectBeforeCollectCallback*/true);
}

#endif // _CHAKRACOREBUILD
26 changes: 26 additions & 0 deletions lib/Runtime/Library/JavascriptLibrary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5300,6 +5300,32 @@ namespace Js
this->nativeHostPromiseContinuationFunctionState = state;
}

void JavascriptLibrary::SetNativeHostPromiseRejectionTrackerCallback(HostPromiseRejectionTrackerCallback function, void *state)
{
this->nativeHostPromiseRejectionTracker = function;
this->nativeHostPromiseContinuationFunctionState = state;
}

void JavascriptLibrary::CallNativeHostPromiseRejectionTracker(Var promise, Var reason, bool handled)
{
if (this->nativeHostPromiseRejectionTracker != nullptr)
{
BEGIN_LEAVE_SCRIPT(scriptContext);
try
{
this->nativeHostPromiseRejectionTracker(promise, reason, handled, this->nativeHostPromiseContinuationFunctionState);
}
catch (...)
{
// Hosts are required not to pass exceptions back across the callback boundary. If
// this happens, it is a bug in the host, not something that we are expected to
// handle gracefully.
Js::Throw::FatalInternalError();
}
END_LEAVE_SCRIPT(scriptContext);
}
}

void JavascriptLibrary::SetJsrtContext(FinalizableObject* jsrtContext)
{
// With JsrtContext supporting cross context, ensure that it doesn't get GCed
Expand Down
6 changes: 6 additions & 0 deletions lib/Runtime/Library/JavascriptLibrary.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ namespace Js
static DWORD GetRandSeed1Offset() { return offsetof(JavascriptLibrary, randSeed1); }
static DWORD GetTypeDisplayStringsOffset() { return offsetof(JavascriptLibrary, typeDisplayStrings); }
typedef bool (CALLBACK *PromiseContinuationCallback)(Var task, void *callbackState);
typedef void (CALLBACK *HostPromiseRejectionTrackerCallback)(Var promise, Var reason, bool handled, void *callbackState);

Var GetUndeclBlockVar() const { return undeclBlockVarSentinel; }
bool IsUndeclBlockVar(Var var) const { return var == undeclBlockVarSentinel; }
Expand Down Expand Up @@ -492,6 +493,9 @@ namespace Js
FieldNoBarrier(PromiseContinuationCallback) nativeHostPromiseContinuationFunction;
Field(void *) nativeHostPromiseContinuationFunctionState;

FieldNoBarrier(HostPromiseRejectionTrackerCallback) nativeHostPromiseRejectionTracker = nullptr;
Field(void *) nativeHostPromiseRejectionTrackerState;

typedef SList<Js::FunctionProxy*, Recycler> FunctionReferenceList;
typedef JsUtil::WeakReferenceDictionary<uintptr_t, DynamicType, DictionarySizePolicy<PowerOf2Policy, 1>> JsrtExternalTypesCache;

Expand Down Expand Up @@ -949,6 +953,8 @@ namespace Js
JavascriptFunction* GetThrowerFunction() const { return throwerFunction; }

void SetNativeHostPromiseContinuationFunction(PromiseContinuationCallback function, void *state);
void SetNativeHostPromiseRejectionTrackerCallback(HostPromiseRejectionTrackerCallback function, void *state);
void CallNativeHostPromiseRejectionTracker(Var promise, Var reason, bool handled);

void SetJsrtContext(FinalizableObject* jsrtContext);
FinalizableObject* GetJsrtContext();
Expand Down
11 changes: 11 additions & 0 deletions lib/Runtime/Library/JavascriptPromise.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ namespace Js
Assert(type->GetTypeId() == TypeIds_Promise);

this->status = PromiseStatusCode_Undefined;
this->isHandled = false;
this->result = nullptr;
this->resolveReactions = nullptr;
this->rejectReactions = nullptr;
Expand Down Expand Up @@ -660,6 +661,10 @@ namespace Js
{
reactions = this->GetRejectReactions();
newStatus = PromiseStatusCode_HasRejection;
if (!GetIsHandled())
{
scriptContext->GetLibrary()->CallNativeHostPromiseRejectionTracker(this, resolution, false);
}
}
else
{
Expand Down Expand Up @@ -838,13 +843,19 @@ namespace Js
EnqueuePromiseReactionTask(resolveReaction, sourcePromise->result, scriptContext);
break;
case PromiseStatusCode_HasRejection:
if (!sourcePromise->GetIsHandled())
{
scriptContext->GetLibrary()->CallNativeHostPromiseRejectionTracker(sourcePromise, sourcePromise->result, true);
}
EnqueuePromiseReactionTask(rejectReaction, sourcePromise->result, scriptContext);
break;
default:
AssertMsg(false, "Promise status is in an invalid state");
break;
}

sourcePromise->SetIsHandled();

return promiseCapability->GetPromise();
}

Expand Down
3 changes: 3 additions & 0 deletions lib/Runtime/Library/JavascriptPromise.h
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,8 @@ namespace Js
PromiseStatusCode_HasRejection
};

bool GetIsHandled() { return isHandled; }
void SetIsHandled() { isHandled = true; }
PromiseStatus GetStatus() const { return status; }
Var GetResult() const { return result; }

Expand All @@ -469,6 +471,7 @@ namespace Js

protected:
Field(PromiseStatus) status;
Field(bool) isHandled;
Field(Var) result;
Field(JavascriptPromiseReactionList*) resolveReactions;
Field(JavascriptPromiseReactionList*) rejectReactions;
Expand Down
43 changes: 43 additions & 0 deletions test/es7/PromiseRejectionTracking.baseline
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
Executing test #1 - Reject promise with no reactions.
Uncaught promise rejection
Rejection from test 1
Executing test #2 - Reject promise with a catch reaction only.
Executing test #3 - Reject promise with catch and then reactions.
Executing test #4 - Reject promise then add a catch afterwards.
Uncaught promise rejection
Rejection from test 4
Promise rejection handled
Rejection from test 4
Executing test #5 - Reject promise then add two catches afterwards.
Uncaught promise rejection
Rejection from test 5
Promise rejection handled
Rejection from test 5
Executing test #6 - Async function that throws.
Uncaught promise rejection
Rejection from test 6
Executing test #7 - Async function that throws but is caught.
Uncaught promise rejection
Rejection from test 7
Promise rejection handled
Rejection from test 7
Executing test #8 - Async function that awaits a function that throws.
Uncaught promise rejection
Rejection from test 8
Promise rejection handled
Rejection from test 8
Executing test #9 - Reject a handled promise then handle one of the handles but not the other.
Executing test #10 - Reject a handled promise and don't handle either path.
Begin async results:
Uncaught promise rejection
Rejection from test 8
Uncaught promise rejection
Rejection from test 9
Uncaught promise rejection
Rejection from test 10
Uncaught promise rejection
Rejection from test 10
Promise rejection handled
Rejection from test 9
Uncaught promise rejection
Rejection from test 9
Loading