diff --git a/deps/chakrashim/include/v8.h b/deps/chakrashim/include/v8.h index 3c5299b4b4d..40816dddfa3 100644 --- a/deps/chakrashim/include/v8.h +++ b/deps/chakrashim/include/v8.h @@ -1448,9 +1448,6 @@ class EXPORT Isolate { static Isolate* New(); static Isolate* GetCurrent(); - typedef bool (*abort_on_uncaught_exception_t)(); - void SetAbortOnUncaughtException(abort_on_uncaught_exception_t callback); - void Enter(); void Exit(); void Dispose(); diff --git a/deps/chakrashim/src/jsrtcachedpropertyidref.inc b/deps/chakrashim/src/jsrtcachedpropertyidref.inc index 1661ef66538..bc5f1b0a2de 100644 --- a/deps/chakrashim/src/jsrtcachedpropertyidref.inc +++ b/deps/chakrashim/src/jsrtcachedpropertyidref.inc @@ -42,7 +42,6 @@ DEF(ownKeys) DEF(preventExtensions) DEF(set) DEF(setPrototypeOf) -DEF(__keepalive__) DEF(length) DEF(writable) DEF(configurable) @@ -77,6 +76,7 @@ DEFSYMBOL(self) DEFSYMBOL(__external__) DEFSYMBOL(__hiddenvalues__) DEFSYMBOL(__isexternal__) +DEFSYMBOL(__keepalive__) #undef DEF #undef DEFSYMBOL diff --git a/deps/chakrashim/src/jsrtisolateshim.cc b/deps/chakrashim/src/jsrtisolateshim.cc index 8a0e1157188..df8633c6581 100644 --- a/deps/chakrashim/src/jsrtisolateshim.cc +++ b/deps/chakrashim/src/jsrtisolateshim.cc @@ -113,6 +113,8 @@ bool IsolateShim::Dispose() { // Set the current IsolateShim scope v8::Isolate::Scope scope(ToIsolate(this)); if (JsDisposeRuntime(runtime) != JsNoError) { + // Can't do much at this point. Assert that this doesn't happen in debug + CHAKRA_ASSERT(false); return false; } } @@ -135,8 +137,8 @@ bool IsolateShim::IsDisposing() { return isDisposing; } -//TODO: Chakra: This is not called after cross context work in chakra. Fix this else we will -// leak chakrashim object. +// CHAKRA-TODO: This is not called after cross context work in chakra. Fix this +// else we will leak chakrashim object. void CALLBACK IsolateShim::JsContextBeforeCollectCallback( _In_ JsRef contextRef, _In_opt_ void *data) { IsolateShim * isolateShim = reinterpret_cast(data); @@ -185,7 +187,6 @@ void IsolateShim::DisposeAll() { IsolateShim * curr = s_isolateList; s_isolateList = nullptr; while (curr) { - // CHAKRA-TODO: Handle error? curr->Dispose(); curr = curr->next; } @@ -202,10 +203,10 @@ void IsolateShim::PushScope( scope->previous = this->contextScopeStack; this->contextScopeStack = scope; - // CHAKRA-TODO: Error handling? - JsSetCurrentContext(contextShim->GetContextRef()); + // Don't crash even if we fail to set the context + JsErrorCode errorCode = JsSetCurrentContext(contextShim->GetContextRef()); + CHAKRA_ASSERT(errorCode == JsNoError); - // CHAKRA-TODO: Error handling? if (!contextShim->EnsureInitialized()) { Fatal("Failed to initialize context"); } @@ -215,7 +216,6 @@ void IsolateShim::PopScope(ContextShim::Scope * scope) { assert(this->contextScopeStack == scope); ContextShim::Scope * prevScope = scope->previous; if (prevScope != nullptr) { - // Marshal the pending exception JsValueRef exception = JS_INVALID_REFERENCE; bool hasException; if (scope->contextShim != prevScope->contextShim && @@ -224,9 +224,12 @@ void IsolateShim::PopScope(ContextShim::Scope * scope) { JsGetAndClearException(&exception) == JsNoError) { } - JsSetCurrentContext(prevScope->contextShim->GetContextRef()); + // Don't crash even if we fail to set the context + JsErrorCode errorCode = JsSetCurrentContext( + prevScope->contextShim->GetContextRef()); + CHAKRA_ASSERT(errorCode == JsNoError); - // CHAKRA-TODO: Error handling? + // Propagate the exception to parent scope if (exception != JS_INVALID_REFERENCE) { JsSetException(exception); } @@ -245,9 +248,7 @@ JsPropertyIdRef IsolateShim::GetSelfSymbolPropertyIdRef() { } JsPropertyIdRef IsolateShim::GetKeepAliveObjectSymbolPropertyIdRef() { - // CHAKRA-TODO: has a bug with symbols and proxy, just a real property name - return GetCachedPropertyIdRef(CachedPropertyIdRef::__keepalive__); - // return EnsurePrivateSymbol(&keepAliveObjectSymbolPropertyIdRef); + return GetCachedSymbolPropertyIdRef(CachedSymbolPropertyIdRef::__keepalive__); } template @@ -296,8 +297,7 @@ JsPropertyIdRef IsolateShim::GetProxyTrapPropertyIdRef(ProxyTraps trap) { ContextShim * IsolateShim::GetContextShimOfObject(JsValueRef valueRef) { JsContextRef contextRef; - if (JsGetContextOfObject(valueRef, &contextRef) != JsNoError) - { + if (JsGetContextOfObject(valueRef, &contextRef) != JsNoError) { return nullptr; } assert(contextRef != nullptr); @@ -327,7 +327,7 @@ bool IsolateShim::AddMessageListener(void * that) { try { messageListeners.push_back(that); return true; - } catch (...) { + } catch(...) { return false; } } diff --git a/deps/chakrashim/src/jsrtutils.cc b/deps/chakrashim/src/jsrtutils.cc index 3d9bc704e72..57242d3513d 100644 --- a/deps/chakrashim/src/jsrtutils.cc +++ b/deps/chakrashim/src/jsrtutils.cc @@ -897,16 +897,4 @@ void Fatal(const char * format, ...) { abort(); } - -void SetOutOfMemoryErrorIfExist(_In_ JsErrorCode errorCode) { - if (errorCode == JsErrorOutOfMemory) { - const wchar_t txt[] = L"process out of memory"; - JsValueRef msg, err; - if (JsPointerToString(txt, _countof(txt) - 1, &msg) == JsNoError && - JsCreateError(msg, &err) == JsNoError) { - JsSetException(err); - } - } -} - } // namespace jsrt diff --git a/deps/chakrashim/src/jsrtutils.h b/deps/chakrashim/src/jsrtutils.h index dc6702d7aa6..3ffa5fb04bc 100644 --- a/deps/chakrashim/src/jsrtutils.h +++ b/deps/chakrashim/src/jsrtutils.h @@ -319,11 +319,6 @@ class JsArguments { } }; -// Check if the errorCode was JsErrorOutOfMemory and if yes, -// create and set the exception on the context -void SetOutOfMemoryErrorIfExist(_In_ JsErrorCode errorCode); - - template Function::Call( JsValueRef result; { TryCatch tryCatch; - JsErrorCode error = - JsCallFunction((JsValueRef)this, args.get(), argc + 1, &result); - - jsrt::SetOutOfMemoryErrorIfExist(error); - - if (error == JsNoError) { - return Local::New(static_cast(result)); - } - if (error != JsErrorInvalidArgument) { + if (JsCallFunction((JsValueRef)this, args.get(), + argc + 1, &result) != JsNoError) { tryCatch.CheckReportExternalException(); return Local(); } - } - - // Invalid argument may mean some of the object are from another context - // Check and marshal and call again. - - // NOTE: Ideally, we will never run into this situation where we will - // also marshal correctly and use object from the same context. - // But CopyProperty in node_contextify.cc violate that so, we have this - // to paper over it. - IsolateShim * isolateShim = IsolateShim::GetCurrent(); - ContextShim * currentContextShim = isolateShim->GetCurrentContextShim(); - if (currentContextShim == nullptr) { - return Local(); - } - - for (int i = 0; i < argc + 1; i++) { - JsValueRef valueRef = args.get()[i]; - ContextShim * objectContextShim = isolateShim->GetContextShimOfObject(valueRef); - if (currentContextShim == objectContextShim) { - continue; - } - if (objectContextShim != nullptr) { - ContextShim::Scope scope(objectContextShim); - args.get()[i] = valueRef; - } else { - // Can't find a context - return Local(); - } - } - - { - TryCatch tryCatch; - JsErrorCode error = JsCallFunction((JsValueRef)this, - args.get(), argc + 1, &result); - - jsrt::SetOutOfMemoryErrorIfExist(error); - - if (error != JsNoError) { - tryCatch.CheckReportExternalException(); - return Local(); - } - return Local::New(static_cast(result)); + return Local::New(static_cast(result)); } } @@ -131,7 +83,7 @@ void Function::SetName(Handle name) { (JsValueRef)*name, JS_INVALID_REFERENCE, JS_INVALID_REFERENCE); - // CHAKRA-TODO: Check error? + CHAKRA_ASSERT(error == JsNoError); } Function *Function::Cast(Value *obj) { diff --git a/deps/chakrashim/src/v8handlescope.cc b/deps/chakrashim/src/v8handlescope.cc index 78aa65fdd99..093364bbf0a 100644 --- a/deps/chakrashim/src/v8handlescope.cc +++ b/deps/chakrashim/src/v8handlescope.cc @@ -20,6 +20,7 @@ #include "v8.h" #include "jsrt.h" +#include "jsrtutils.h" namespace v8 { @@ -41,8 +42,9 @@ HandleScope::~HandleScope() { AddRefRecord * currRecord = this->_addRefRecordHead; while (currRecord != nullptr) { AddRefRecord * nextRecord = currRecord->_next; - // CHAKRA-TODO: Report error? - JsRelease(currRecord->_ref, nullptr); + // Don't crash even if we fail to release the scope + JsErrorCode errorCode = JsRelease(currRecord->_ref, nullptr); + CHAKRA_ASSERT(errorCode == JsNoError); delete currRecord; currRecord = nextRecord; } diff --git a/deps/chakrashim/src/v8isolate.cc b/deps/chakrashim/src/v8isolate.cc index 8ba48091b8b..19f54c25d9c 100644 --- a/deps/chakrashim/src/v8isolate.cc +++ b/deps/chakrashim/src/v8isolate.cc @@ -35,11 +35,6 @@ Isolate *Isolate::GetCurrent() { return jsrt::IsolateShim::GetCurrentAsIsolate(); } -void Isolate::SetAbortOnUncaughtException( - abort_on_uncaught_exception_t callback) { - // CHAKRA-TODO -} - void Isolate::Enter() { return jsrt::IsolateShim::FromIsolate(this)->Enter(); } @@ -50,7 +45,6 @@ void Isolate::Exit() { } void Isolate::Dispose() { - // CHAKRA-TODO: Handle Error? jsrt::IsolateShim::FromIsolate(this)->Dispose(); } diff --git a/deps/chakrashim/src/v8script.cc b/deps/chakrashim/src/v8script.cc index fb2f08e4943..55ca3874330 100644 --- a/deps/chakrashim/src/v8script.cc +++ b/deps/chakrashim/src/v8script.cc @@ -91,9 +91,6 @@ Local