* Update per 21114-part2 changes. * Add explicit thread safety guards. * Add Entities.queryPropertyMetdata for unit testing Entity script module support. * Cleanup / commenting pass.

humbletim committed 9 years ago

Commit 40ba8181 parent 9b096518 files · +439 −158Browse files

Showing the changes to libraries/shared/src/BaseScriptEngine.cpp (1 of 8 files). Show all

Unified · Split

libraries/shared/src/BaseScriptEngine.cpp+101−28

 //
 #include "BaseScriptEngine.h"
+#include "SharedLogging.h"
 #include <QtCore/QString>
 #include <QtCore/QThread>
 #include <QtScript/QScriptValueIterator>
 #include <QtScript/QScriptContextInfo>
-#include "ScriptEngineLogging.h"
 #include "Profile.h"
-const QString BaseScriptEngine::_SETTINGS_ENABLE_EXTENDED_EXCEPTIONS {
-    "com.highfidelity.experimental.enableExtendedJSExceptions"
-};
 const QString BaseScriptEngine::SCRIPT_EXCEPTION_FORMAT { "[%0] %1 in %2:%3" };
 const QString BaseScriptEngine::SCRIPT_BACKTRACE_SEP { "\n    " };
+bool BaseScriptEngine::IS_THREADSAFE_INVOCATION(const QThread *thread, const QString& method) {
+    if (QThread::currentThread() == thread) {
+        return true;
+    }
+    qCCritical(shared) << QString("Scripting::%1 @ %2 -- ignoring thread-unsafe call from %3")
+        .arg(method).arg(thread ? thread->objectName() : "(!thread)").arg(QThread::currentThread()->objectName());
+    qCDebug(shared) << "(please resolve on the calling side by using invokeMethod, executeOnScriptThread, etc.)";
+    return false;
+}
 // engine-aware JS Error copier and factory
 QScriptValue BaseScriptEngine::makeError(const QScriptValue& _other, const QString& type) {
+    if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
+        return unboundNullValue();
+    }
     auto other = _other;
     if (other.isString()) {
         other = newObject();
     }
     if (!proto.isFunction()) {
 #ifdef DEBUG_JS_EXCEPTIONS
-        qCDebug(scriptengine) << "BaseScriptEngine::makeError -- couldn't find constructor for" << type << " -- using Error instead";
+        qCDebug(shared) << "BaseScriptEngine::makeError -- couldn't find constructor for" << type << " -- using Error instead";
 #endif
         proto = globalObject().property("Error");
     }
 // check syntax and when there are issues returns an actual "SyntaxError" with the details
 QScriptValue BaseScriptEngine::lintScript(const QString& sourceCode, const QString& fileName, const int lineNumber) {
+    if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
+        return unboundNullValue();
+    }
     const auto syntaxCheck = checkSyntax(sourceCode);
     if (syntaxCheck.state() != QScriptSyntaxCheckResult::Valid) {
         auto err = globalObject().property("SyntaxError")
         }
         return err;
     }
-    return undefinedValue();
+    return QScriptValue();
 }
 // this pulls from the best available information to create a detailed snapshot of the current exception
 QScriptValue BaseScriptEngine::cloneUncaughtException(const QString& extraDetail) {
+    if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
+        return unboundNullValue();
+    }
     if (!hasUncaughtException()) {
-        return QScriptValue();
+        return unboundNullValue();
     }
     auto exception = uncaughtException();
     // ensure the error object is engine-local
     return err;
 }
-QString BaseScriptEngine::formatException(const QScriptValue& exception) {
+QString BaseScriptEngine::formatException(const QScriptValue& exception, bool includeExtendedDetails) {
+    if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
+        return QString();
+    }
     QString note { "UncaughtException" };
     QString result;
     const auto lineNumber = exception.property("lineNumber").toString();
     const auto stacktrace =  exception.property("stack").toString();
-    if (_enableExtendedJSExceptions.get()) {
+    if (includeExtendedDetails) {
-        // This setting toggles display of the hints now being added during the loading process.
+        // Display additional exception / troubleshooting hints that can be added via the custom Error .detail property
         // Example difference:
         //   [UncaughtExceptions] Error: Can't find variable: foobar in atp:/myentity.js\n...
         //   [UncaughtException (construct {1eb5d3fa-23b1-411c-af83-163af7220e3f})] Error: Can't find variable: foobar in atp:/myentity.js\n...
     return result;
 }
+bool BaseScriptEngine::raiseException(const QScriptValue& exception) {
+    if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
+        return false;
+    }
+    if (currentContext()) {
+        // we have an active context / JS stack frame so throw the exception per usual
+        currentContext()->throwValue(makeError(exception));
+        return true;
+    } else {
+        // we are within a pure C++ stack frame (ie: being called directly by other C++ code)
+        // in this case no context information is available so just emit the exception for reporting
+        emit unhandledException(makeError(exception));
+    }
+    return false;
+}
+bool BaseScriptEngine::maybeEmitUncaughtException(const QString& debugHint) {
+    if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
+        return false;
+    }
+    if (!isEvaluating() && hasUncaughtException()) {
+        emit unhandledException(cloneUncaughtException(debugHint));
+        clearExceptions();
+        return true;
+    }
+    return false;
+}
 QScriptValue BaseScriptEngine::evaluateInClosure(const QScriptValue& closure, const QScriptProgram& program) {
     PROFILE_RANGE(script, "evaluateInClosure");
-    if (QThread::currentThread() != thread()) {
+    if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
-        qCCritical(scriptengine) << "*** CRITICAL *** ScriptEngine::evaluateInClosure() is meant to be called from engine thread only.";
+        return unboundNullValue();
-        // note: a recursive mutex might be needed around below code if this method ever becomes Q_INVOKABLE
-        return QScriptValue();
     }
     const auto fileName = program.fileName();
     const auto shortName = QUrl(fileName).fileName();
     auto global = closure.property("global");
     if (global.isObject()) {
 #ifdef DEBUG_JS
-        qCDebug(scriptengine) << " setting global = closure.global" << shortName;
+        qCDebug(shared) << " setting global = closure.global" << shortName;
 #endif
         oldGlobal = globalObject();
         setGlobalObject(global);
     auto thiz = closure.property("this");
     if (thiz.isObject()) {
 #ifdef DEBUG_JS
-        qCDebug(scriptengine) << " setting this = closure.this" << shortName;
+        qCDebug(shared) << " setting this = closure.this" << shortName;
 #endif
         context->setThisObject(thiz);
     }
     context->pushScope(closure);
 #ifdef DEBUG_JS
-    qCDebug(scriptengine) << QString("[%1] evaluateInClosure %2").arg(isEvaluating()).arg(shortName);
+    qCDebug(shared) << QString("[%1] evaluateInClosure %2").arg(isEvaluating()).arg(shortName);
 #endif
     {
         result = BaseScriptEngine::evaluate(program);
         if (hasUncaughtException()) {
             auto err = cloneUncaughtException(__FUNCTION__);
 #ifdef DEBUG_JS_EXCEPTIONS
-            qCWarning(scriptengine) << __FUNCTION__ << "---------- hasCaught:" << err.toString() << result.toString();
+            qCWarning(shared) << __FUNCTION__ << "---------- hasCaught:" << err.toString() << result.toString();
             err.setProperty("_result", result);
 #endif
             result = err;
         }
     }
 #ifdef DEBUG_JS
-    qCDebug(scriptengine) << QString("[%1] //evaluateInClosure %2").arg(isEvaluating()).arg(shortName);
+    qCDebug(shared) << QString("[%1] //evaluateInClosure %2").arg(isEvaluating()).arg(shortName);
 #endif
     popContext();
     if (oldGlobal.isValid()) {
 #ifdef DEBUG_JS
-        qCDebug(scriptengine) << " restoring global" << shortName;
+        qCDebug(shared) << " restoring global" << shortName;
 #endif
         setGlobalObject(oldGlobal);
     }
 }
 // Lambda
 QScriptValue BaseScriptEngine::newLambdaFunction(std::function<QScriptValue(QScriptContext *, QScriptEngine*)> operation, const QScriptValue& data, const QScriptEngine::ValueOwnership& ownership) {
     auto lambda = new Lambda(this, operation, data);
     auto object = newQObject(lambda, ownership);
 #endif
 }
 QScriptValue Lambda::call() {
+    if (!BaseScriptEngine::IS_THREADSAFE_INVOCATION(engine->thread(), __FUNCTION__)) {
+        return BaseScriptEngine::unboundNullValue();
+    }
     return operation(engine->currentContext(), engine);
 }
+QScriptValue makeScopedHandlerObject(QScriptValue scopeOrCallback, QScriptValue methodOrName) {
+    auto engine = scopeOrCallback.engine();
+    if (!engine) {
+        return scopeOrCallback;
+    }
+    auto scope = QScriptValue();
+    auto callback = scopeOrCallback;
+    if (scopeOrCallback.isObject()) {
+        if (methodOrName.isString()) {
+            scope = scopeOrCallback;
+            callback = scope.property(methodOrName.toString());
+        } else if (methodOrName.isFunction()) {
+            scope = scopeOrCallback;
+            callback = methodOrName;
+        }
+    }
+    auto handler = engine->newObject();
+    handler.setProperty("scope", scope);
+    handler.setProperty("callback", callback);
+    return handler;
+}
+QScriptValue callScopedHandlerObject(QScriptValue handler, QScriptValue err, QScriptValue result) {
+    return handler.property("callback").call(handler.property("scope"), QScriptValueList({ err, result }));
+}
 #ifdef DEBUG_JS
 void BaseScriptEngine::_debugDump(const QString& header, const QScriptValue& object, const QString& footer) {
+    if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
+        return;
+    }
     if (!header.isEmpty()) {
-        qCDebug(scriptengine) << header;
+        qCDebug(shared) << header;
     }
     if (!object.isObject()) {
-        qCDebug(scriptengine) << "(!isObject)" << object.toVariant().toString() << object.toString();
+        qCDebug(shared) << "(!isObject)" << object.toVariant().toString() << object.toString();
         return;
     }
     QScriptValueIterator it(object);
     while (it.hasNext()) {
         it.next();
-        qCDebug(scriptengine) << it.name() << ":" << it.value().toString();
+        qCDebug(shared) << it.name() << ":" << it.value().toString();
     }
     if (!footer.isEmpty()) {
-        qCDebug(scriptengine) << footer;
+        qCDebug(shared) << footer;
     }
 }
 #endif