diff options
author | Max Hirschhorn <max.hirschhorn@mongodb.com> | 2018-09-18 21:10:36 -0400 |
---|---|---|
committer | Max Hirschhorn <max.hirschhorn@mongodb.com> | 2018-09-18 21:10:36 -0400 |
commit | f43ea7a6bb2a6c44f91506c32004241d3cbb24ae (patch) | |
tree | 1dff1f481a3439045f0de84a242e1ac33f201249 /src/mongo/scripting/mozjs | |
parent | 78112be586c67efa877636d596b194650e90cbed (diff) | |
download | mongo-f43ea7a6bb2a6c44f91506c32004241d3cbb24ae.tar.gz |
SERVER-35154 Propagate JS exceptions through ScopedThread#join().
This makes it so that if the ScopedThread exited due to an uncaught
JavaScript exception, then calling .join() or .returnData() on it throws
a JavaScript exception with the error message and stacktrace intact.
Diffstat (limited to 'src/mongo/scripting/mozjs')
-rw-r--r-- | src/mongo/scripting/mozjs/implscope.cpp | 60 | ||||
-rw-r--r-- | src/mongo/scripting/mozjs/jsthread.cpp | 27 | ||||
-rw-r--r-- | src/mongo/scripting/mozjs/status.cpp | 44 | ||||
-rw-r--r-- | src/mongo/scripting/mozjs/status.h | 1 |
4 files changed, 100 insertions, 32 deletions
diff --git a/src/mongo/scripting/mozjs/implscope.cpp b/src/mongo/scripting/mozjs/implscope.cpp index bf35606f2ba..c852899aa46 100644 --- a/src/mongo/scripting/mozjs/implscope.cpp +++ b/src/mongo/scripting/mozjs/implscope.cpp @@ -41,11 +41,13 @@ #include "mongo/db/server_parameters.h" #include "mongo/platform/decimal128.h" #include "mongo/platform/stack_locator.h" +#include "mongo/scripting/jsexception.h" #include "mongo/scripting/mozjs/objectwrapper.h" #include "mongo/scripting/mozjs/valuereader.h" #include "mongo/scripting/mozjs/valuewriter.h" #include "mongo/stdx/memory.h" #include "mongo/stdx/mutex.h" +#include "mongo/util/assert_util.h" #include "mongo/util/log.h" #include "mongo/util/scopeguard.h" @@ -125,10 +127,6 @@ void MozJSImplScope::_reportError(JSContext* cx, const char* message, JSErrorRep std::string exceptionMsg; try { - str::stream ss; - - ss << message; - // TODO: something far more elaborate that mimics the stack printing from v8 JS::RootedValue excn(cx); if (JS_GetPendingException(cx, &excn) && excn.isObject()) { @@ -138,23 +136,24 @@ void MozJSImplScope::_reportError(JSContext* cx, const char* message, JSErrorRep ObjectWrapper o(cx, obj); o.getValue("stack", &stack); - auto str = ValueWriter(cx, stack).toString(); + auto stackStr = ValueWriter(cx, stack).toString(); - if (str.empty()) { - ss << " @" << report->filename << ":" << report->lineno << ":" << report->column + if (stackStr.empty()) { + str::stream ss; + ss << "@" << report->filename << ":" << report->lineno << ":" << report->column << "\n"; - } else { - ss << " :\n" << str; + stackStr = ss; } - scope->_status = + auto status = jsExceptionToStatus(cx, excn, ErrorCodes::JSInterpreterFailure, message) - .withReason(ss); + .withReason(message); + scope->_status = {JSExceptionInfo(std::move(stackStr), std::move(status)), message}; return; } - exceptionMsg = ss; + exceptionMsg = message; } catch (const DBException& dbe) { exceptionMsg = "Unknown error occured while processing exception"; log() << exceptionMsg << ":" << dbe.toString() << ":" << message; @@ -526,12 +525,28 @@ auto MozJSImplScope::_runSafely(ImplScopeFunction&& functionToRun) -> decltype(f MozJSEntry entry(this); return functionToRun(); } catch (...) { + // There may have already been an error reported by SpiderMonkey. If not, then we use the + // active C++ exception as the cause of the error. + if (_status.isOK()) { + _status = exceptionToStatus(); + } + + if (auto extraInfo = _status.extraInfo<JSExceptionInfo>()) { + // We intentionally don't transmit an JSInterpreterFailureWithStack error over the wire + // because of the complexity it'd entail on the recipient to reach inside to the + // underlying error for how it should be handled. Instead, the error is unwrapped and + // the JavaScript stacktrace is included as part of the error message. + str::stream reasonWithStack; + reasonWithStack << extraInfo->originalError.reason() << " :\n" << extraInfo->stack; + _status = extraInfo->originalError.withReason(reasonWithStack); + } + _error = _status.reason(); // Clear the status state auto status = std::move(_status); uassertStatusOK(status); - throw; + MONGO_UNREACHABLE; } } @@ -609,6 +624,10 @@ void MozJSImplScope::_MozJSCreateFunction(StringData raw, JS::MutableHandleValue } BSONObj MozJSImplScope::callThreadArgs(const BSONObj& args) { + // The _runSafely() function is called for all codepaths of executing JavaScript other than + // callThreadArgs(). We intentionally don't unwrap the JSInterpreterFailureWithStack error to + // make it possible for the parent thread to chain its JavaScript stacktrace with the child + // thread's JavaScript stacktrace. MozJSEntry entry(this); JS::RootedValue function(_context); @@ -903,16 +922,21 @@ bool MozJSImplScope::_checkErrorState(bool success, bool reportError, bool asser ss << "[" << fnameStr << ":" << lineNum << ":" << colNum << "] "; } ss << ValueWriter(_context, excn).toString(); - if (stackStr != "") { - ss << "\nStack trace:\n" << stackStr << "----------\n"; - } - _status = Status(ErrorCodes::JSInterpreterFailure, ss); + _status = { + JSExceptionInfo(std::move(stackStr), Status(ErrorCodes::JSInterpreterFailure, ss)), + ss}; } else { _status = Status(ErrorCodes::UnknownError, "Unknown Failure from JSInterpreter"); } } - _error = _status.reason(); + if (auto extraInfo = _status.extraInfo<JSExceptionInfo>()) { + str::stream reasonWithStack; + reasonWithStack << _status.reason() << " :\n" << extraInfo->stack; + _error = reasonWithStack; + } else { + _error = _status.reason(); + } if (reportError) error() << redact(_error); diff --git a/src/mongo/scripting/mozjs/jsthread.cpp b/src/mongo/scripting/mozjs/jsthread.cpp index 5dccaf36bae..0072aa419e0 100644 --- a/src/mongo/scripting/mozjs/jsthread.cpp +++ b/src/mongo/scripting/mozjs/jsthread.cpp @@ -124,6 +124,8 @@ public: PR_JoinThread(_thread); _done = true; + + uassertStatusOK(_sharedData->getErrorStatus()); } /** @@ -134,7 +136,7 @@ public: bool hasFailed() const { uassert(ErrorCodes::JSInterpreterFailure, "Thread not started", _started); - return _sharedData->getErrored(); + return !_sharedData->getErrorStatus().isOK(); } BSONObj returnData() { @@ -154,16 +156,16 @@ private: */ class SharedData { public: - SharedData() : _errored(false) {} + SharedData() = default; - void setErrored(bool value) { - stdx::lock_guard<stdx::mutex> lck(_erroredMutex); - _errored = value; + void setErrorStatus(Status status) { + stdx::lock_guard<stdx::mutex> lck(_statusMutex); + _status = std::move(status); } - bool getErrored() { - stdx::lock_guard<stdx::mutex> lck(_erroredMutex); - return _errored; + Status getErrorStatus() { + stdx::lock_guard<stdx::mutex> lck(_statusMutex); + return _status; } /** @@ -176,8 +178,8 @@ private: std::string _stack; private: - stdx::mutex _erroredMutex; - bool _errored; + stdx::mutex _statusMutex; + Status _status = Status::OK(); }; /** @@ -197,10 +199,7 @@ private: thisv->_sharedData->_returnData = scope.callThreadArgs(thisv->_sharedData->_args); } catch (...) { auto status = exceptionToStatus(); - - log() << "js thread raised js exception: " << redact(status) - << thisv->_sharedData->_stack; - thisv->_sharedData->setErrored(true); + thisv->_sharedData->setErrorStatus(status); thisv->_sharedData->_returnData = BSON("ret" << BSONUndefined); } } diff --git a/src/mongo/scripting/mozjs/status.cpp b/src/mongo/scripting/mozjs/status.cpp index cb4ecb1bbd3..a78238d03ce 100644 --- a/src/mongo/scripting/mozjs/status.cpp +++ b/src/mongo/scripting/mozjs/status.cpp @@ -30,6 +30,7 @@ #include "mongo/scripting/mozjs/status.h" +#include "mongo/scripting/jsexception.h" #include "mongo/scripting/mozjs/implscope.h" #include "mongo/scripting/mozjs/internedstring.h" #include "mongo/scripting/mozjs/objectwrapper.h" @@ -76,6 +77,14 @@ void MongoStatusInfo::fromStatus(JSContext* cx, Status status, JS::MutableHandle JSPROP_ENUMERATE | JSPROP_SHARED, smUtils::wrapConstrainedMethod<Functions::reason, false, MongoStatusInfo>); + // We intentionally omit JSPROP_ENUMERATE to match how Error.prototype.stack is a non-enumerable + // property. + thisvObj.defineProperty( + InternedString::stack, + undef, + JSPROP_SHARED, + smUtils::wrapConstrainedMethod<Functions::stack, false, MongoStatusInfo>); + JS_SetPrivate(thisv, scope->trackedNew<Status>(std::move(status))); value.setObjectOrNull(thisv); @@ -106,6 +115,41 @@ void MongoStatusInfo::Functions::reason::call(JSContext* cx, JS::CallArgs args) ValueReader(cx, args.rval()).fromStringData(toStatus(cx, args.thisv()).reason()); } +void MongoStatusInfo::Functions::stack::call(JSContext* cx, JS::CallArgs args) { + JS::RootedObject thisv(cx, args.thisv().toObjectOrNull()); + JS::RootedObject parent(cx); + + if (!JS_GetPrototype(cx, thisv, &parent)) { + uasserted(ErrorCodes::JSInterpreterFailure, "Couldn't get prototype"); + } + + ObjectWrapper parentWrapper(cx, parent); + + auto status = toStatus(cx, args.thisv()); + if (auto extraInfo = status.extraInfo<JSExceptionInfo>()) { + // 'status' represents an uncaught JavaScript exception that was handled in C++. It is + // expected to have been thrown by a JSThread. We chain its stacktrace together with the + // stacktrace of the JavaScript exception in the current thread in order to show more + // context about the latter's cause. + JS::RootedValue stack(cx); + ValueReader(cx, &stack) + .fromStringData(extraInfo->stack + parentWrapper.getString(InternedString::stack)); + + // We redefine the "stack" property as the combined JavaScript stacktrace. It is important + // that we omit JSPROP_SHARED to the thisvObj.defineProperty() call in order to have + // SpiderMonkey allocate memory for the string value. We also intentionally omit + // JSPROP_ENUMERATE to match how Error.prototype.stack is a non-enumerable property. + ObjectWrapper thisvObj(cx, args.thisv()); + thisvObj.defineProperty(InternedString::stack, stack, 0U); + + // We intentionally use thisvObj.getValue() to access the "stack" property to implicitly + // verify it has been redefined correctly, as the alternative would be infinite recursion. + thisvObj.getValue(InternedString::stack, args.rval()); + } else { + parentWrapper.getValue(InternedString::stack, args.rval()); + } +} + void MongoStatusInfo::postInstall(JSContext* cx, JS::HandleObject global, JS::HandleObject proto) { auto scope = getScope(cx); diff --git a/src/mongo/scripting/mozjs/status.h b/src/mongo/scripting/mozjs/status.h index d930bdb8df9..8a066936520 100644 --- a/src/mongo/scripting/mozjs/status.h +++ b/src/mongo/scripting/mozjs/status.h @@ -51,6 +51,7 @@ struct MongoStatusInfo : public BaseInfo { struct Functions { MONGO_DECLARE_JS_FUNCTION(code); MONGO_DECLARE_JS_FUNCTION(reason); + MONGO_DECLARE_JS_FUNCTION(stack); }; static void postInstall(JSContext* cx, JS::HandleObject global, JS::HandleObject proto); |