diff --git a/libraries/animation/src/Rig.cpp b/libraries/animation/src/Rig.cpp index a57f0b01d2..9e2ba46ddb 100644 --- a/libraries/animation/src/Rig.cpp +++ b/libraries/animation/src/Rig.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include "AnimationLogging.h" @@ -796,23 +797,35 @@ void Rig::computeMotionAnimationState(float deltaTime, const glm::vec3& worldPos // Allow script to add/remove handlers and report results, from within their thread. QScriptValue Rig::addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { // called in script thread - QMutexLocker locker(&_stateMutex); - // Find a safe id, even if there are lots of many scripts add and remove handlers repeatedly. - while (!_nextStateHandlerId || _stateHandlers.contains(_nextStateHandlerId)) { // 0 is unused, and don't reuse existing after wrap. - _nextStateHandlerId++; + + // validate argument types + if (handler.isFunction() && (isListOfStrings(propertiesList) || propertiesList.isUndefined() || propertiesList.isNull())) { + QMutexLocker locker(&_stateMutex); + // Find a safe id, even if there are lots of many scripts add and remove handlers repeatedly. + while (!_nextStateHandlerId || _stateHandlers.contains(_nextStateHandlerId)) { // 0 is unused, and don't reuse existing after wrap. + _nextStateHandlerId++; + } + StateHandler& data = _stateHandlers[_nextStateHandlerId]; + data.function = handler; + data.useNames = propertiesList.isArray(); + if (data.useNames) { + data.propertyNames = propertiesList.toVariant().toStringList(); + } + return QScriptValue(_nextStateHandlerId); // suitable for giving to removeAnimationStateHandler + } else { + qCWarning(animation) << "Rig::addAnimationStateHandler invalid arguments, expected (function, string[])"; + return QScriptValue(QScriptValue::UndefinedValue); } - StateHandler& data = _stateHandlers[_nextStateHandlerId]; - data.function = handler; - data.useNames = propertiesList.isArray(); - if (data.useNames) { - data.propertyNames = propertiesList.toVariant().toStringList(); - } - return QScriptValue(_nextStateHandlerId); // suitable for giving to removeAnimationStateHandler } void Rig::removeAnimationStateHandler(QScriptValue identifier) { // called in script thread - QMutexLocker locker(&_stateMutex); - _stateHandlers.remove(identifier.isNumber() ? identifier.toInt32() : 0); // silently continues if handler not present. 0 is unused + // validate arguments + if (identifier.isNumber()) { + QMutexLocker locker(&_stateMutex); + _stateHandlers.remove(identifier.toInt32()); // silently continues if handler not present. 0 is unused + } else { + qCWarning(animation) << "Rig::removeAnimationStateHandler invalid argument, expected a number"; + } } void Rig::animationStateHandlerResult(int identifier, QScriptValue result) { // called synchronously from script diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index 9611e2ec65..96e05c8d09 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -101,8 +101,7 @@ void ResourceCache::refresh(const QUrl& url) { if (resource) { resource->refresh(); } else { - QWriteLocker locker(&_resourcesLock); - _resources.remove(url); + removeResource(url); resetResourceCounters(); } } @@ -196,11 +195,8 @@ void ResourceCache::addUnusedResource(const QSharedPointer& resource) // If it doesn't fit or its size is unknown, remove it from the cache. if (resource->getBytes() == 0 || resource->getBytes() > _unusedResourcesMaxSize) { resource->setCache(nullptr); - - _totalResourcesSize -= resource->getBytes(); - _resources.remove(resource->getURL()); + removeResource(resource->getURL(), resource->getBytes()); resetResourceCounters(); - return; } reserveUnusedResource(resource->getBytes()); @@ -233,8 +229,7 @@ void ResourceCache::reserveUnusedResource(qint64 resourceSize) { it.value()->setCache(nullptr); auto size = it.value()->getBytes(); - _totalResourcesSize -= size; - _resources.remove(it.value()->getURL()); + removeResource(it.value()->getURL(), size); _unusedResourcesSize -= size; _unusedResources.erase(it); @@ -259,6 +254,12 @@ void ResourceCache::resetResourceCounters() { emit dirty(); } +void ResourceCache::removeResource(const QUrl& url, qint64 size) { + QWriteLocker locker(&_resourcesLock); + _resources.remove(url); + _totalResourcesSize -= size; +} + void ResourceCache::updateTotalSize(const qint64& oldSize, const qint64& newSize) { _totalResourcesSize += (newSize - oldSize); emit dirty(); @@ -454,12 +455,12 @@ void Resource::refresh() { } void Resource::allReferencesCleared() { - if (_cache && isCacheable()) { - if (QThread::currentThread() != thread()) { - QMetaObject::invokeMethod(this, "allReferencesCleared"); - return; - } + if (QThread::currentThread() != thread()) { + QMetaObject::invokeMethod(this, "allReferencesCleared"); + return; + } + if (_cache && isCacheable()) { // create and reinsert new shared pointer QSharedPointer self(this, &Resource::allReferencesCleared); setSelf(self); @@ -467,8 +468,13 @@ void Resource::allReferencesCleared() { // add to the unused list _cache->addUnusedResource(self); - } else { + if (_cache) { + // remove from the cache + _cache->removeResource(getURL(), getBytes()); + _cache->resetResourceCounters(); + } + deleteLater(); } } diff --git a/libraries/networking/src/ResourceCache.h b/libraries/networking/src/ResourceCache.h index aea1e91099..ed3dbf69b6 100644 --- a/libraries/networking/src/ResourceCache.h +++ b/libraries/networking/src/ResourceCache.h @@ -153,6 +153,7 @@ private: void reserveUnusedResource(qint64 resourceSize); void clearUnusedResource(); void resetResourceCounters(); + void removeResource(const QUrl& url, qint64 size = 0); QReadWriteLock _resourcesLock { QReadWriteLock::Recursive }; QHash> _resources; diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index a3e0744b46..506bdb9463 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -784,7 +784,13 @@ void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantM callingArguments << javascriptParameters; assert(currentEntityIdentifier.isInvalidID()); // No animation state handlers from entity scripts. QScriptValue result = callback.call(QScriptValue(), callingArguments); - resultHandler(result); + + // validate result from callback function. + if (result.isValid() && result.isObject()) { + resultHandler(result); + } else { + qCWarning(scriptengine) << "ScriptEngine::callAnimationStateHandler invalid return argument from callback, expected an object"; + } } void ScriptEngine::timerFired() { diff --git a/libraries/shared/src/ScriptValueUtils.cpp b/libraries/shared/src/ScriptValueUtils.cpp new file mode 100644 index 0000000000..e352c0546d --- /dev/null +++ b/libraries/shared/src/ScriptValueUtils.cpp @@ -0,0 +1,34 @@ +// +// ScriptValueUtils.cpp +// libraries/shared/src +// +// Created by Anthony Thibault on 4/15/16. +// Copyright 2016 High Fidelity, Inc. +// +// Utilities for working with QtScriptValues +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#include "ScriptValueUtils.h" + +bool isListOfStrings(const QScriptValue& arg) { + if (!arg.isArray()) { + return false; + } + + auto lengthProperty = arg.property("length"); + if (!lengthProperty.isNumber()) { + return false; + } + + int length = lengthProperty.toInt32(); + for (int i = 0; i < length; i++) { + if (!arg.property(i).isString()) { + return false; + } + } + + return true; +} diff --git a/libraries/shared/src/ScriptValueUtils.h b/libraries/shared/src/ScriptValueUtils.h new file mode 100644 index 0000000000..2e120a7217 --- /dev/null +++ b/libraries/shared/src/ScriptValueUtils.h @@ -0,0 +1,21 @@ +// +// ScriptValueUtils.h +// libraries/shared/src +// +// Created by Anthony Thibault on 4/15/16. +// Copyright 2016 High Fidelity, Inc. +// +// Utilities for working with QtScriptValues +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#ifndef hifi_ScriptValueUtils_h +#define hifi_ScriptValueUtils_h + +#include + +bool isListOfStrings(const QScriptValue& value); + +#endif // #define hifi_ScriptValueUtils_h diff --git a/libraries/shared/src/shared/RateCounter.h b/libraries/shared/src/shared/RateCounter.h index 754439ec15..e24b1e6eca 100644 --- a/libraries/shared/src/shared/RateCounter.h +++ b/libraries/shared/src/shared/RateCounter.h @@ -22,10 +22,10 @@ class RateCounter { public: void increment(size_t count = 1) { auto now = usecTimestampNow(); - auto currentIntervalMs = (uint32_t)((now - _start) / USECS_PER_MSEC); - if (currentIntervalMs > INTERVAL) { + float currentIntervalMs = (now - _start) / (float) USECS_PER_MSEC; + if (currentIntervalMs > (float) INTERVAL) { float currentCount = _count; - float intervalSeconds = (float)currentIntervalMs / (float)MSECS_PER_SECOND; + float intervalSeconds = currentIntervalMs / (float) MSECS_PER_SECOND; _rate = roundf(currentCount / intervalSeconds * _scale) / _scale; _start = now; _count = 0;