From 316d533d19c858ca309fe21654c48427447ae1bb Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 11 Apr 2016 13:50:42 -0700 Subject: [PATCH 01/40] Update crash reporting to cover a larger area --- interface/src/CrashReporter.cpp | 137 ++++++++++++++++++++++++++++++++ interface/src/CrashReporter.h | 32 ++++++++ interface/src/main.cpp | 113 +++++++++++++++++++++++--- 3 files changed, 270 insertions(+), 12 deletions(-) create mode 100644 interface/src/CrashReporter.cpp create mode 100644 interface/src/CrashReporter.h diff --git a/interface/src/CrashReporter.cpp b/interface/src/CrashReporter.cpp new file mode 100644 index 0000000000..022866ce4b --- /dev/null +++ b/interface/src/CrashReporter.cpp @@ -0,0 +1,137 @@ +// +// CrashReporter.cpp +// interface/src +// +// Created by Ryan Huffman on 11 April 2016. +// Copyright 2016 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + + +#ifdef HAS_BUGSPLAT +#include "CrashReporter.h" +#include +#include +#include +#include + +// SetUnhandledExceptionFilter can be overridden by the CRT at the point that an error occurs. More information +// can be found here: http://www.codeproject.com/Articles/154686/SetUnhandledExceptionFilter-and-the-C-C-Runtime-Li +// A fairly common approach is to patch the SetUnhandledExceptionFilter so that it cannot be overridden and so +// that the applicaiton can handle it itself. +// The CAPIHook class referenced in the above article is not openly available, but a similar implementation +// can be found here: http://blog.kalmbach-software.de/2008/04/02/unhandled-exceptions-in-vc8-and-above-for-x86-and-x64/ +// The below has been adapted to work with different library and funcitons. +BOOL redirectLibraryFunctionToFunction(char* library, char* function, void* fn) +{ + HMODULE lib = LoadLibrary(library); + if (lib == NULL) return FALSE; + void *pOrgEntry = GetProcAddress(lib, function); + if (pOrgEntry == NULL) return FALSE; + + DWORD dwOldProtect = 0; + SIZE_T jmpSize = 5; +#ifdef _M_X64 + jmpSize = 13; +#endif + BOOL bProt = VirtualProtect(pOrgEntry, jmpSize, + PAGE_EXECUTE_READWRITE, &dwOldProtect); + BYTE newJump[20]; + void *pNewFunc = fn; +#ifdef _M_IX86 + DWORD dwOrgEntryAddr = (DWORD)pOrgEntry; + dwOrgEntryAddr += jmpSize; // add 5 for 5 op-codes for jmp rel32 + DWORD dwNewEntryAddr = (DWORD)pNewFunc; + DWORD dwRelativeAddr = dwNewEntryAddr - dwOrgEntryAddr; + // JMP rel32: Jump near, relative, displacement relative to next instruction. + newJump[0] = 0xE9; // JMP rel32 + memcpy(&newJump[1], &dwRelativeAddr, sizeof(pNewFunc)); +#elif _M_X64 + // We must use R10 or R11, because these are "scratch" registers + // which need not to be preserved accross function calls + // For more info see: Register Usage for x64 64-Bit + // http://msdn.microsoft.com/en-us/library/ms794547.aspx + // Thanks to Matthew Smith!!! + newJump[0] = 0x49; // MOV R11, ... + newJump[1] = 0xBB; // ... + memcpy(&newJump[2], &pNewFunc, sizeof(pNewFunc)); + //pCur += sizeof (ULONG_PTR); + newJump[10] = 0x41; // JMP R11, ... + newJump[11] = 0xFF; // ... + newJump[12] = 0xE3; // ... +#endif + SIZE_T bytesWritten; + BOOL bRet = WriteProcessMemory(GetCurrentProcess(), + pOrgEntry, newJump, jmpSize, &bytesWritten); + + if (bProt != FALSE) + { + DWORD dwBuf; + VirtualProtect(pOrgEntry, jmpSize, dwOldProtect, &dwBuf); + } + return bRet; +} + + +void handleSignal(int signal) { + // Throw so BugSplat can handle + throw(signal); +} + +void handlePureVirtualCall() { + // Throw so BugSplat can handle + throw("ERROR: Pure virtual call"); +} + +void handleInvalidParameter(const wchar_t * expression, const wchar_t * function, const wchar_t * file, + unsigned int line, uintptr_t pReserved ) { + // Throw so BugSplat can handle + throw("ERROR: Invalid parameter"); +} + +int handleNewError(size_t size) { + // Throw so BugSplat can handle + throw("ERROR: Errors calling new"); +} + +LPTOP_LEVEL_EXCEPTION_FILTER WINAPI +noop_SetUnhandledExceptionFilter( + LPTOP_LEVEL_EXCEPTION_FILTER lpTopLevelExceptionFilter) +{ + return nullptr; +} + +_purecall_handler __cdecl +noop_set_purecall_handler(_purecall_handler pNew) +{ + return nullptr; +} + +static const DWORD BUG_SPLAT_FLAGS = MDSF_PREVENTHIJACKING | MDSF_USEGUARDMEMORY; + +CrashReporter::CrashReporter(QString bugSplatDatabase, QString bugSplatApplicationName, QString version) + : mpSender(qPrintable(bugSplatDatabase), qPrintable(bugSplatApplicationName), qPrintable(version), nullptr, BUG_SPLAT_FLAGS) +{ + signal(SIGSEGV, handleSignal); + signal(SIGABRT, handleSignal); + _set_purecall_handler(handlePureVirtualCall); + _set_invalid_parameter_handler(handleInvalidParameter); + _set_new_mode(1); + _set_new_handler(handleNewError); + + // Disable WER popup +// SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX | SEM_NOOPENFILEERRORBOX); +// _set_abort_behavior(0, _WRITE_ABORT_MSG | _CALL_REPORTFAULT); + + // QtWebEngineCore internally sets its own purecall handler, overriding our own error handling. This disables that. + if (!redirectLibraryFunctionToFunction("msvcr120.dll", "_set_purecall_handler", &noop_set_purecall_handler)) { + qWarning() << "Failed to patch _set_purecall_handler"; + } + // Patch SetUnhandledExceptionFilter to keep the CRT from overriding our own error handling. + if (!redirectLibraryFunctionToFunction("kernel32.dll", "SetUnhandledExceptionFilter", &noop_SetUnhandledExceptionFilter)) { + qWarning() << "Failed to patch setUnhandledExceptionFilter"; + } +} +#endif diff --git a/interface/src/CrashReporter.h b/interface/src/CrashReporter.h new file mode 100644 index 0000000000..5f02066a74 --- /dev/null +++ b/interface/src/CrashReporter.h @@ -0,0 +1,32 @@ +// +// CrashReporter.h +// interface/src +// +// Created by Ryan Huffman on 11 April 2016. +// Copyright 2016 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#pragma once + +#ifndef hifi_CrashReporter_h +#define hifi_CrashReporter_h + +#include + +#ifdef HAS_BUGSPLAT + +#include + +class CrashReporter { +public: + CrashReporter(QString bugSplatDatabase, QString bugSplatApplicationName, QString version); + + MiniDmpSender mpSender; +}; + +#endif + +#endif // hifi_CrashReporter_h \ No newline at end of file diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 9c07912b99..d2d346072d 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -25,25 +25,101 @@ #include "InterfaceLogging.h" #include "UserActivityLogger.h" #include "MainWindow.h" +#include #ifdef HAS_BUGSPLAT #include #include +#include #endif +#include + +namespace crash { + + void pureVirtualCall() { + struct B { + B() { + qDebug() << "Pure Virtual Function Call crash!"; + Bar(); + } + + virtual void Foo() = 0; + + void Bar() { + Foo(); + } + }; + + struct D : public B { + void Foo() { + qDebug() << "D:Foo()"; + } + }; + + B* b = new D; + qDebug() << "About to make a pure virtual call"; + b->Foo(); + } + + void doubleFree() { + qDebug() << "About to double delete memory"; + int* blah = new int(200); + delete blah; + delete blah; + } + + void nullDeref() { + qDebug() << "About to dereference a null pointer"; + int* p = nullptr; + *p = 1; + } + + void doAbort() { + qDebug() << "About to abort"; + abort(); + } + + void outOfBoundsVectorCrash() { + qDebug() << "std::vector out of bounds crash!"; + std::vector v; + v[0] = 5; + } + + void newFault() { + qDebug() << "About to crash inside new fault"; + // Force crash with large allocation + int *pi = new int[std::numeric_limits::max()]; + } +} +// +//LONG WINAPI TopLevelExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { +// qDebug() << "Got exception"; +// +// mpSender.unhandledExceptionHandler(pExceptionInfo); +// +// return EXCEPTION_CONTINUE_SEARCH; +//} -int main(int argc, const char* argv[]) { - disableQtBearerPoll(); // Fixes wifi ping spikes #if HAS_BUGSPLAT // Prevent other threads from hijacking the Exception filter, and allocate 4MB up-front that may be useful in // low-memory scenarios. - static const DWORD BUG_SPLAT_FLAGS = MDSF_PREVENTHIJACKING | MDSF_USEGUARDMEMORY; - static const char* BUG_SPLAT_DATABASE = "interface_alpha"; - static const char* BUG_SPLAT_APPLICATION_NAME = "Interface"; - MiniDmpSender mpSender { BUG_SPLAT_DATABASE, BUG_SPLAT_APPLICATION_NAME, qPrintable(BuildInfo::VERSION), - nullptr, BUG_SPLAT_FLAGS }; +// static const DWORD BUG_SPLAT_FLAGS = MDSF_PREVENTHIJACKING | MDSF_USEGUARDMEMORY;// | MDSF_CUSTOMEXCEPTIONFILTER; +// static const char* BUG_SPLAT_DATABASE = "interface_alpha"; +// static const char* BUG_SPLAT_APPLICATION_NAME = "Interface"; +// static MiniDmpSender mpSender { BUG_SPLAT_DATABASE, BUG_SPLAT_APPLICATION_NAME, qPrintable(BuildInfo::VERSION), +// nullptr, BUG_SPLAT_FLAGS }; #endif + +int main(int argc, const char* argv[]) { +#if HAS_BUGSPLAT + static QString BUG_SPLAT_DATABASE = "interface_alpha"; + static QString BUG_SPLAT_APPLICATION_NAME = "Interface"; + CrashReporter crashReporter { BUG_SPLAT_DATABASE, BUG_SPLAT_APPLICATION_NAME, BuildInfo::VERSION }; +#endif + + disableQtBearerPoll(); // Fixes wifi ping spikes QString applicationName = "High Fidelity Interface - " + qgetenv("USERNAME"); @@ -160,6 +236,19 @@ int main(int argc, const char* argv[]) { } } +// crash::doAbort(); // works +// crash::doubleFree(); + + std::thread([]() { +// crash::pureVirtualCall(); // works + crash::newFault(); // works + }); + +// QThread thread; +// QObject::connect(&thread, &QThread::started, &app, []() { +// }, Qt::DirectConnection); +// thread.start(); + // Setup local server QLocalServer server { &app }; @@ -167,18 +256,18 @@ int main(int argc, const char* argv[]) { server.removeServer(applicationName); server.listen(applicationName); - QObject::connect(&server, &QLocalServer::newConnection, &app, &Application::handleLocalServerConnection); + QObject::connect(&server, &QLocalServer::newConnection, &app, &Application::handleLocalServerConnection, Qt::DirectConnection); #ifdef HAS_BUGSPLAT AccountManager& accountManager = AccountManager::getInstance(); - mpSender.setDefaultUserName(qPrintable(accountManager.getAccountInfo().getUsername())); - QObject::connect(&accountManager, &AccountManager::usernameChanged, &app, [&mpSender](const QString& newUsername) { - mpSender.setDefaultUserName(qPrintable(newUsername)); + crashReporter.mpSender.setDefaultUserName(qPrintable(accountManager.getAccountInfo().getUsername())); + QObject::connect(&accountManager, &AccountManager::usernameChanged, &app, [&crashReporter](const QString& newUsername) { + crashReporter.mpSender.setDefaultUserName(qPrintable(newUsername)); }); // BugSplat WILL NOT work with file paths that do not use OS native separators. auto logPath = QDir::toNativeSeparators(app.getLogger()->getFilename()); - mpSender.sendAdditionalFile(qPrintable(logPath)); + crashReporter.mpSender.sendAdditionalFile(qPrintable(logPath)); #endif printSystemInformation(); From 63aee23bf12095f48b05386262f8587ff0c8d553 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 11 Apr 2016 14:29:16 -0700 Subject: [PATCH 02/40] Move crash testing to CrashHelpers.h --- interface/src/main.cpp | 79 +--------------------------- libraries/shared/src/CrashHelpers.h | 80 +++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 78 deletions(-) create mode 100644 libraries/shared/src/CrashHelpers.h diff --git a/interface/src/main.cpp b/interface/src/main.cpp index d2d346072d..819fa11323 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -33,84 +33,7 @@ #include #endif -#include - -namespace crash { - - void pureVirtualCall() { - struct B { - B() { - qDebug() << "Pure Virtual Function Call crash!"; - Bar(); - } - - virtual void Foo() = 0; - - void Bar() { - Foo(); - } - }; - - struct D : public B { - void Foo() { - qDebug() << "D:Foo()"; - } - }; - - B* b = new D; - qDebug() << "About to make a pure virtual call"; - b->Foo(); - } - - void doubleFree() { - qDebug() << "About to double delete memory"; - int* blah = new int(200); - delete blah; - delete blah; - } - - void nullDeref() { - qDebug() << "About to dereference a null pointer"; - int* p = nullptr; - *p = 1; - } - - void doAbort() { - qDebug() << "About to abort"; - abort(); - } - - void outOfBoundsVectorCrash() { - qDebug() << "std::vector out of bounds crash!"; - std::vector v; - v[0] = 5; - } - - void newFault() { - qDebug() << "About to crash inside new fault"; - // Force crash with large allocation - int *pi = new int[std::numeric_limits::max()]; - } -} -// -//LONG WINAPI TopLevelExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { -// qDebug() << "Got exception"; -// -// mpSender.unhandledExceptionHandler(pExceptionInfo); -// -// return EXCEPTION_CONTINUE_SEARCH; -//} - - -#if HAS_BUGSPLAT - // Prevent other threads from hijacking the Exception filter, and allocate 4MB up-front that may be useful in - // low-memory scenarios. -// static const DWORD BUG_SPLAT_FLAGS = MDSF_PREVENTHIJACKING | MDSF_USEGUARDMEMORY;// | MDSF_CUSTOMEXCEPTIONFILTER; -// static const char* BUG_SPLAT_DATABASE = "interface_alpha"; -// static const char* BUG_SPLAT_APPLICATION_NAME = "Interface"; -// static MiniDmpSender mpSender { BUG_SPLAT_DATABASE, BUG_SPLAT_APPLICATION_NAME, qPrintable(BuildInfo::VERSION), -// nullptr, BUG_SPLAT_FLAGS }; -#endif +#include int main(int argc, const char* argv[]) { #if HAS_BUGSPLAT diff --git a/libraries/shared/src/CrashHelpers.h b/libraries/shared/src/CrashHelpers.h new file mode 100644 index 0000000000..9146df746c --- /dev/null +++ b/libraries/shared/src/CrashHelpers.h @@ -0,0 +1,80 @@ +// +// CrashHelpers.h +// libraries/shared/src +// +// Created by Ryan Huffman on 11 April 2016. +// Copyright 2016 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#pragma once + +#ifndef hifi_CrashHelpers_h +#define hifi_CrashHelpers_h + +namespace crash { + +class B; +class A { +public: + A(B* b) : _b(b) { } + ~A(); + virtual void virtualFunction() = 0; + +private: + B* _b; +}; + +class B : public A { +public: + B() : A(this) { } + virtual void virtualFunction() { } +}; + +A::~A() { + _b->virtualFunction(); +} + +void pureVirtualCall() { + + qDebug() << "About to make a pure virtual call"; + { + B b; + } +} + +void doubleFree() { + qDebug() << "About to double delete memory"; + int* blah = new int(200); + delete blah; + delete blah; +} + +void nullDeref() { + qDebug() << "About to dereference a null pointer"; + int* p = nullptr; + *p = 1; +} + +void doAbort() { + qDebug() << "About to abort"; + abort(); +} + +void outOfBoundsVectorCrash() { + qDebug() << "std::vector out of bounds crash!"; + std::vector v; + v[0] = 42; +} + +void newFault() { + qDebug() << "About to crash inside new fault"; + // Force crash with large allocation + int *crash = new int[std::numeric_limits::max()]; +} + +} + +#endif // hifi_CrashHelpers_h From 9faeb13ae993e882c9fd8f3ce541a8e8e6ea6ce3 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 11 Apr 2016 14:38:19 -0700 Subject: [PATCH 03/40] Remove error testing code --- interface/src/main.cpp | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 819fa11323..5ff20fe1eb 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -159,19 +159,6 @@ int main(int argc, const char* argv[]) { } } -// crash::doAbort(); // works -// crash::doubleFree(); - - std::thread([]() { -// crash::pureVirtualCall(); // works - crash::newFault(); // works - }); - -// QThread thread; -// QObject::connect(&thread, &QThread::started, &app, []() { -// }, Qt::DirectConnection); -// thread.start(); - // Setup local server QLocalServer server { &app }; From 02a17c4497ae5297807ea180a2183b679a7ad8ff Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 11 Apr 2016 14:38:29 -0700 Subject: [PATCH 04/40] Fix commented out code --- interface/src/CrashReporter.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/src/CrashReporter.cpp b/interface/src/CrashReporter.cpp index 022866ce4b..3a7e45af5e 100644 --- a/interface/src/CrashReporter.cpp +++ b/interface/src/CrashReporter.cpp @@ -122,8 +122,8 @@ CrashReporter::CrashReporter(QString bugSplatDatabase, QString bugSplatApplicati _set_new_handler(handleNewError); // Disable WER popup -// SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX | SEM_NOOPENFILEERRORBOX); -// _set_abort_behavior(0, _WRITE_ABORT_MSG | _CALL_REPORTFAULT); + //SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX | SEM_NOOPENFILEERRORBOX); + //_set_abort_behavior(0, _WRITE_ABORT_MSG | _CALL_REPORTFAULT); // QtWebEngineCore internally sets its own purecall handler, overriding our own error handling. This disables that. if (!redirectLibraryFunctionToFunction("msvcr120.dll", "_set_purecall_handler", &noop_set_purecall_handler)) { From afe502e1d22d56842906df11612ab852f4bb38a6 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 11 Apr 2016 14:43:05 -0700 Subject: [PATCH 05/40] Fix typo --- interface/src/CrashReporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/CrashReporter.cpp b/interface/src/CrashReporter.cpp index 3a7e45af5e..797a9d0587 100644 --- a/interface/src/CrashReporter.cpp +++ b/interface/src/CrashReporter.cpp @@ -23,7 +23,7 @@ // that the applicaiton can handle it itself. // The CAPIHook class referenced in the above article is not openly available, but a similar implementation // can be found here: http://blog.kalmbach-software.de/2008/04/02/unhandled-exceptions-in-vc8-and-above-for-x86-and-x64/ -// The below has been adapted to work with different library and funcitons. +// The below has been adapted to work with different library and functions. BOOL redirectLibraryFunctionToFunction(char* library, char* function, void* fn) { HMODULE lib = LoadLibrary(library); From 2dfe96a654761fc474c7a2ce0aab31e25c1faef7 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 11 Apr 2016 14:50:28 -0700 Subject: [PATCH 06/40] Cleanup headers --- interface/src/CrashReporter.cpp | 7 +++++-- interface/src/main.cpp | 2 -- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/interface/src/CrashReporter.cpp b/interface/src/CrashReporter.cpp index 797a9d0587..de8782e279 100644 --- a/interface/src/CrashReporter.cpp +++ b/interface/src/CrashReporter.cpp @@ -12,9 +12,12 @@ #ifdef HAS_BUGSPLAT #include "CrashReporter.h" -#include -#include + #include +#include + +#include + #include // SetUnhandledExceptionFilter can be overridden by the CRT at the point that an error occurs. More information diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 5ff20fe1eb..7000e5f905 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -33,8 +33,6 @@ #include #endif -#include - int main(int argc, const char* argv[]) { #if HAS_BUGSPLAT static QString BUG_SPLAT_DATABASE = "interface_alpha"; From afdfef148200c5256266c9c01b6e3f00ab0a050e Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Sun, 10 Apr 2016 21:54:49 -0700 Subject: [PATCH 07/40] Make sure we don't use raw resource ptr --- .../GlobalServicesScriptingInterface.cpp | 4 +- interface/src/ui/Stats.cpp | 11 ++-- libraries/animation/src/AnimNodeLoader.cpp | 13 ++--- libraries/animation/src/AnimNodeLoader.h | 2 +- libraries/networking/src/ResourceCache.cpp | 51 ++++++++++--------- libraries/networking/src/ResourceCache.h | 35 +++++++------ .../src/AnimInverseKinematicsTests.cpp | 1 - tests/networking/src/ResourceTests.cpp | 12 +++-- 8 files changed, 71 insertions(+), 58 deletions(-) diff --git a/interface/src/scripting/GlobalServicesScriptingInterface.cpp b/interface/src/scripting/GlobalServicesScriptingInterface.cpp index 7dac0247bd..d56a987d2b 100644 --- a/interface/src/scripting/GlobalServicesScriptingInterface.cpp +++ b/interface/src/scripting/GlobalServicesScriptingInterface.cpp @@ -122,8 +122,8 @@ void DownloadInfoResultFromScriptValue(const QScriptValue& object, DownloadInfoR DownloadInfoResult GlobalServicesScriptingInterface::getDownloadInfo() { DownloadInfoResult result; - foreach(Resource* resource, ResourceCache::getLoadingRequests()) { - result.downloading.append(resource->getProgress() * 100.0f); + foreach(QSharedPointer resource, ResourceCache::getLoadingRequests()) { + result.downloading.append(resource ? (resource->getProgress() * 100.0f) : 0.0f); } result.pending = ResourceCache::getPendingRequestCount(); return result; diff --git a/interface/src/ui/Stats.cpp b/interface/src/ui/Stats.cpp index 39a3a1f192..d19f7e2cab 100644 --- a/interface/src/ui/Stats.cpp +++ b/interface/src/ui/Stats.cpp @@ -196,7 +196,7 @@ void Stats::updateStats(bool force) { STAT_UPDATE(audioMixerPps, -1); } - QList loadingRequests = ResourceCache::getLoadingRequests(); + QList> loadingRequests = ResourceCache::getLoadingRequests(); STAT_UPDATE(downloads, loadingRequests.size()); STAT_UPDATE(downloadLimit, ResourceCache::getRequestLimit()) STAT_UPDATE(downloadsPending, ResourceCache::getPendingRequestCount()); @@ -205,7 +205,8 @@ void Stats::updateStats(bool force) { bool shouldUpdateUrls = _downloads != _downloadUrls.size(); if (!shouldUpdateUrls) { for (int i = 0; i < _downloads; i++) { - if (loadingRequests[i]->getURL().toString() != _downloadUrls[i]) { + auto request = loadingRequests[i].lock(); + if (!request || request->getURL().toString() != _downloadUrls[i]) { shouldUpdateUrls = true; break; } @@ -214,8 +215,10 @@ void Stats::updateStats(bool force) { // If the urls have changed, update the list if (shouldUpdateUrls) { _downloadUrls.clear(); - foreach (Resource* resource, loadingRequests) { - _downloadUrls << resource->getURL().toString(); + foreach (QSharedPointer request, loadingRequests) { + if (request) { + _downloadUrls << request->getURL().toString(); + } } emit downloadUrlsChanged(); } diff --git a/libraries/animation/src/AnimNodeLoader.cpp b/libraries/animation/src/AnimNodeLoader.cpp index 70cf7e248f..df39c55e5d 100644 --- a/libraries/animation/src/AnimNodeLoader.cpp +++ b/libraries/animation/src/AnimNodeLoader.cpp @@ -568,12 +568,13 @@ bool processStateMachineNode(AnimNode::Pointer node, const QJsonObject& jsonObj, } AnimNodeLoader::AnimNodeLoader(const QUrl& url) : - _url(url), - _resource(nullptr) { - - _resource = new Resource(url); - connect(_resource, &Resource::loaded, this, &AnimNodeLoader::onRequestDone); - connect(_resource, &Resource::failed, this, &AnimNodeLoader::onRequestError); + _url(url) +{ + _resource = QSharedPointer::create(url); + _resource->setSelf(_resource); + connect(_resource.data(), &Resource::loaded, this, &AnimNodeLoader::onRequestDone); + connect(_resource.data(), &Resource::failed, this, &AnimNodeLoader::onRequestError); + _resource->ensureLoading(); } AnimNode::Pointer AnimNodeLoader::load(const QByteArray& contents, const QUrl& jsonUrl) { diff --git a/libraries/animation/src/AnimNodeLoader.h b/libraries/animation/src/AnimNodeLoader.h index d6fdfa7e2c..a1bd4a1f17 100644 --- a/libraries/animation/src/AnimNodeLoader.h +++ b/libraries/animation/src/AnimNodeLoader.h @@ -41,7 +41,7 @@ protected slots: protected: QUrl _url; - Resource* _resource; + QSharedPointer _resource; private: // no copies diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index 2273902263..11d0f31217 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -110,7 +110,7 @@ QSharedPointer ResourceCache::getResource(const QUrl& url, const QUrl& } if (QThread::currentThread() != thread()) { - assert(delayLoad); + assert(delayLoad && !extra); getResourceAsynchronously(url); return QSharedPointer(); } @@ -198,17 +198,17 @@ void ResourceCache::updateTotalSize(const qint64& oldSize, const qint64& newSize emit dirty(); } -void ResourceCacheSharedItems::appendActiveRequest(Resource* resource) { +void ResourceCacheSharedItems::appendActiveRequest(QWeakPointer resource) { Lock lock(_mutex); _loadingRequests.append(resource); } -void ResourceCacheSharedItems::appendPendingRequest(Resource* resource) { +void ResourceCacheSharedItems::appendPendingRequest(QWeakPointer resource) { Lock lock(_mutex); _pendingRequests.append(resource); } -QList> ResourceCacheSharedItems::getPendingRequests() const { +QList> ResourceCacheSharedItems::getPendingRequests() const { Lock lock(_mutex); return _pendingRequests; } @@ -218,23 +218,24 @@ uint32_t ResourceCacheSharedItems::getPendingRequestsCount() const { return _pendingRequests.size(); } -QList ResourceCacheSharedItems::getLoadingRequests() const { +QList> ResourceCacheSharedItems::getLoadingRequests() const { Lock lock(_mutex); return _loadingRequests; } -void ResourceCacheSharedItems::removeRequest(Resource* resource) { +void ResourceCacheSharedItems::removeRequest(QWeakPointer resource) { Lock lock(_mutex); - _loadingRequests.removeOne(resource); + _loadingRequests.removeAll(resource); } -Resource* ResourceCacheSharedItems::getHighestPendingRequest() { +QSharedPointer ResourceCacheSharedItems::getHighestPendingRequest() { Lock lock(_mutex); // look for the highest priority pending request int highestIndex = -1; float highestPriority = -FLT_MAX; + QSharedPointer highestResource; for (int i = 0; i < _pendingRequests.size();) { - Resource* resource = _pendingRequests.at(i).data(); + auto resource = _pendingRequests.at(i).lock(); if (!resource) { _pendingRequests.removeAt(i); continue; @@ -243,16 +244,25 @@ Resource* ResourceCacheSharedItems::getHighestPendingRequest() { if (priority >= highestPriority) { highestPriority = priority; highestIndex = i; + highestResource = resource; } i++; } if (highestIndex >= 0) { - return _pendingRequests.takeAt(highestIndex); + _pendingRequests.takeAt(highestIndex); } - return nullptr; + return highestResource; } -bool ResourceCache::attemptRequest(Resource* resource) { +const QList> ResourceCache::getLoadingRequests() { + return DependencyManager::get()->getLoadingRequests(); +} + +int ResourceCache::getPendingRequestCount() { + return DependencyManager::get()->getPendingRequestsCount(); +} + +bool ResourceCache::attemptRequest(QSharedPointer resource) { auto sharedItems = DependencyManager::get(); if (_requestsActive >= _requestLimit) { @@ -267,7 +277,7 @@ bool ResourceCache::attemptRequest(Resource* resource) { return true; } -void ResourceCache::requestCompleted(Resource* resource) { +void ResourceCache::requestCompleted(QSharedPointer resource) { auto sharedItems = DependencyManager::get(); sharedItems->removeRequest(resource); --_requestsActive; @@ -291,11 +301,6 @@ Resource::Resource(const QUrl& url, bool delayLoad) : _request(nullptr) { init(); - - // start loading immediately unless instructed otherwise - if (!(_startedLoading || delayLoad)) { - QTimer::singleShot(0, this, &Resource::ensureLoading); - } } Resource::~Resource() { @@ -303,7 +308,7 @@ Resource::~Resource() { _request->disconnect(this); _request->deleteLater(); _request = nullptr; - ResourceCache::requestCompleted(this); + ResourceCache::requestCompleted(_self); } } @@ -356,7 +361,7 @@ void Resource::refresh() { _request->disconnect(this); _request->deleteLater(); _request = nullptr; - ResourceCache::requestCompleted(this); + ResourceCache::requestCompleted(_self); } init(); @@ -401,7 +406,7 @@ void Resource::init() { void Resource::attemptRequest() { _startedLoading = true; - ResourceCache::attemptRequest(this); + ResourceCache::attemptRequest(_self); } void Resource::finishedLoading(bool success) { @@ -433,7 +438,7 @@ void Resource::makeRequest() { if (!_request) { qCDebug(networking).noquote() << "Failed to get request for" << _url.toDisplayString(); - ResourceCache::requestCompleted(this); + ResourceCache::requestCompleted(_self); finishedLoading(false); return; } @@ -465,7 +470,7 @@ void Resource::handleReplyFinished() { return; } - ResourceCache::requestCompleted(this); + ResourceCache::requestCompleted(_self); auto result = _request->getResult(); if (result == ResourceRequest::Success) { diff --git a/libraries/networking/src/ResourceCache.h b/libraries/networking/src/ResourceCache.h index 84eba1cdc0..034ca74882 100644 --- a/libraries/networking/src/ResourceCache.h +++ b/libraries/networking/src/ResourceCache.h @@ -62,21 +62,20 @@ class ResourceCacheSharedItems : public Dependency { using Mutex = std::mutex; using Lock = std::unique_lock; public: - void appendPendingRequest(Resource* newRequest); - void appendActiveRequest(Resource* newRequest); - void removeRequest(Resource* doneRequest); - QList> getPendingRequests() const; + void appendPendingRequest(QWeakPointer newRequest); + void appendActiveRequest(QWeakPointer newRequest); + void removeRequest(QWeakPointer doneRequest); + QList> getPendingRequests() const; uint32_t getPendingRequestsCount() const; - QList getLoadingRequests() const; - Resource* getHighestPendingRequest(); + QList> getLoadingRequests() const; + QSharedPointer getHighestPendingRequest(); private: - ResourceCacheSharedItems() { } - virtual ~ResourceCacheSharedItems() { } + ResourceCacheSharedItems() = default; mutable Mutex _mutex; - QList> _pendingRequests; - QList _loadingRequests; + QList> _pendingRequests; + QList> _loadingRequests; }; @@ -89,6 +88,8 @@ class ResourceCache : public QObject { Q_PROPERTY(size_t sizeCached READ getSizeCachedResources NOTIFY dirty) public: + virtual ~ResourceCache(); + size_t getNumTotalResources() const { return _numTotalResources; } size_t getSizeTotalResources() const { return _totalResourcesSize; } @@ -105,14 +106,10 @@ public: void setUnusedResourceCacheSize(qint64 unusedResourcesMaxSize); qint64 getUnusedResourceCacheSize() const { return _unusedResourcesMaxSize; } - static const QList getLoadingRequests() - { return DependencyManager::get()->getLoadingRequests(); } + static const QList> getLoadingRequests(); - static int getPendingRequestCount() - { return DependencyManager::get()->getPendingRequestsCount(); } + static int getPendingRequestCount(); - ResourceCache(QObject* parent = NULL); - virtual ~ResourceCache(); void refreshAll(); void refresh(const QUrl& url); @@ -127,6 +124,8 @@ protected slots: void updateTotalSize(const qint64& oldSize, const qint64& newSize); protected: + ResourceCache(QObject* parent = nullptr); + /// Loads a resource from the specified URL. /// \param fallback a fallback URL to load if the desired one is unavailable /// \param delayLoad if true, don't load the resource immediately; wait until load is first requested @@ -143,8 +142,8 @@ protected: /// Attempt to load a resource if requests are below the limit, otherwise queue the resource for loading /// \return true if the resource began loading, otherwise false if the resource is in the pending queue - static bool attemptRequest(Resource* resource); - static void requestCompleted(Resource* resource); + static bool attemptRequest(QSharedPointer resource); + static void requestCompleted(QSharedPointer resource); static bool attemptHighestPriorityRequest(); private: diff --git a/tests/animation/src/AnimInverseKinematicsTests.cpp b/tests/animation/src/AnimInverseKinematicsTests.cpp index 2b10892f82..f36b8891ff 100644 --- a/tests/animation/src/AnimInverseKinematicsTests.cpp +++ b/tests/animation/src/AnimInverseKinematicsTests.cpp @@ -11,7 +11,6 @@ #include -#include #include #include #include diff --git a/tests/networking/src/ResourceTests.cpp b/tests/networking/src/ResourceTests.cpp index f18f676398..e6dcf18230 100644 --- a/tests/networking/src/ResourceTests.cpp +++ b/tests/networking/src/ResourceTests.cpp @@ -34,7 +34,7 @@ void ResourceTests::initTestCase() { networkAccessManager.setCache(cache); } -static Resource* resource = nullptr; +static QSharedPointer resource; static bool waitForSignal(QObject *sender, const char *signal, int timeout = 1000) { @@ -55,7 +55,8 @@ void ResourceTests::downloadFirst() { // download the Mery fst file QUrl meryUrl = QUrl("http://hifi-public.s3.amazonaws.com/marketplace/contents/e21c0b95-e502-4d15-8c41-ea2fc40f1125/3585ddf674869a67d31d5964f7b52de1.fst"); - resource = new Resource(meryUrl, false); + resource = QSharedPointer::create(meryUrl, false); + resource->setSelf(resource); const int timeout = 1000; QEventLoop loop; @@ -67,6 +68,8 @@ void ResourceTests::downloadFirst() { loop.connect(resource, SIGNAL(failed(QNetworkReply::NetworkError)), SLOT(quit())); loop.connect(&timer, SIGNAL(timeout()), SLOT(quit())); timer.start(); + + resource->ensureLoading(); loop.exec(); QVERIFY(resource->isLoaded()); @@ -76,7 +79,8 @@ void ResourceTests::downloadAgain() { // download the Mery fst file QUrl meryUrl = QUrl("http://hifi-public.s3.amazonaws.com/marketplace/contents/e21c0b95-e502-4d15-8c41-ea2fc40f1125/3585ddf674869a67d31d5964f7b52de1.fst"); - resource = new Resource(meryUrl, false); + resource = QSharedPointer::create(meryUrl, false); + resource->setSelf(resource); const int timeout = 1000; QEventLoop loop; @@ -88,6 +92,8 @@ void ResourceTests::downloadAgain() { loop.connect(resource, SIGNAL(failed(QNetworkReply::NetworkError)), SLOT(quit())); loop.connect(&timer, SIGNAL(timeout()), SLOT(quit())); timer.start(); + + resource->ensureLoading(); loop.exec(); QVERIFY(resource->isLoaded()); From 5eb4f63573b163742e8a2f57612b0281820fe61b Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Sun, 10 Apr 2016 23:26:06 -0700 Subject: [PATCH 08/40] Use ResourceManager in anim loader --- libraries/animation/src/AnimNodeLoader.cpp | 20 ++++++++++++++------ libraries/animation/src/AnimNodeLoader.h | 8 ++++---- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/libraries/animation/src/AnimNodeLoader.cpp b/libraries/animation/src/AnimNodeLoader.cpp index df39c55e5d..f5d9215253 100644 --- a/libraries/animation/src/AnimNodeLoader.cpp +++ b/libraries/animation/src/AnimNodeLoader.cpp @@ -570,11 +570,19 @@ bool processStateMachineNode(AnimNode::Pointer node, const QJsonObject& jsonObj, AnimNodeLoader::AnimNodeLoader(const QUrl& url) : _url(url) { - _resource = QSharedPointer::create(url); - _resource->setSelf(_resource); - connect(_resource.data(), &Resource::loaded, this, &AnimNodeLoader::onRequestDone); - connect(_resource.data(), &Resource::failed, this, &AnimNodeLoader::onRequestError); - _resource->ensureLoading(); + auto request = ResourceManager::createResourceRequest(this, url); + if (request) { + connect(request, &ResourceRequest::finished, this, [this, request]() { + if (request->getResult() == ResourceRequest::Success) { + onRequestDone(request->getData()); + } else { + onRequestError(request->getResult()); + } + request->deleteLater(); + }); + + request->send(); + } } AnimNode::Pointer AnimNodeLoader::load(const QByteArray& contents, const QUrl& jsonUrl) { @@ -621,6 +629,6 @@ void AnimNodeLoader::onRequestDone(const QByteArray data) { } } -void AnimNodeLoader::onRequestError(QNetworkReply::NetworkError netError) { +void AnimNodeLoader::onRequestError(ResourceRequest::Result netError) { emit error((int)netError, "Resource download error"); } diff --git a/libraries/animation/src/AnimNodeLoader.h b/libraries/animation/src/AnimNodeLoader.h index a1bd4a1f17..edb4729753 100644 --- a/libraries/animation/src/AnimNodeLoader.h +++ b/libraries/animation/src/AnimNodeLoader.h @@ -15,11 +15,11 @@ #include #include -#include #include "AnimNode.h" -class Resource; +#include + class AnimNodeLoader : public QObject { Q_OBJECT @@ -37,11 +37,11 @@ protected: protected slots: void onRequestDone(const QByteArray data); - void onRequestError(QNetworkReply::NetworkError error); + void onRequestError(ResourceRequest::Result error); protected: QUrl _url; - QSharedPointer _resource; + private: // no copies From e45939d18f5e4d8bda5232a44c4e25d090344ae3 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Sun, 10 Apr 2016 23:48:49 -0700 Subject: [PATCH 09/40] Make sure ResourceCacheSharedItems deals with strong refs --- .../GlobalServicesScriptingInterface.cpp | 4 +- interface/src/ui/Stats.cpp | 11 ++--- libraries/networking/src/ResourceCache.cpp | 42 ++++++++++++++----- libraries/networking/src/ResourceCache.h | 12 +++--- 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/interface/src/scripting/GlobalServicesScriptingInterface.cpp b/interface/src/scripting/GlobalServicesScriptingInterface.cpp index d56a987d2b..e6d87a0260 100644 --- a/interface/src/scripting/GlobalServicesScriptingInterface.cpp +++ b/interface/src/scripting/GlobalServicesScriptingInterface.cpp @@ -122,8 +122,8 @@ void DownloadInfoResultFromScriptValue(const QScriptValue& object, DownloadInfoR DownloadInfoResult GlobalServicesScriptingInterface::getDownloadInfo() { DownloadInfoResult result; - foreach(QSharedPointer resource, ResourceCache::getLoadingRequests()) { - result.downloading.append(resource ? (resource->getProgress() * 100.0f) : 0.0f); + foreach(auto resource, ResourceCache::getLoadingRequests()) { + result.downloading.append(resource->getProgress() * 100.0f); } result.pending = ResourceCache::getPendingRequestCount(); return result; diff --git a/interface/src/ui/Stats.cpp b/interface/src/ui/Stats.cpp index d19f7e2cab..d1cb4cf3c2 100644 --- a/interface/src/ui/Stats.cpp +++ b/interface/src/ui/Stats.cpp @@ -196,7 +196,7 @@ void Stats::updateStats(bool force) { STAT_UPDATE(audioMixerPps, -1); } - QList> loadingRequests = ResourceCache::getLoadingRequests(); + auto loadingRequests = ResourceCache::getLoadingRequests(); STAT_UPDATE(downloads, loadingRequests.size()); STAT_UPDATE(downloadLimit, ResourceCache::getRequestLimit()) STAT_UPDATE(downloadsPending, ResourceCache::getPendingRequestCount()); @@ -205,8 +205,7 @@ void Stats::updateStats(bool force) { bool shouldUpdateUrls = _downloads != _downloadUrls.size(); if (!shouldUpdateUrls) { for (int i = 0; i < _downloads; i++) { - auto request = loadingRequests[i].lock(); - if (!request || request->getURL().toString() != _downloadUrls[i]) { + if (loadingRequests[i]->getURL().toString() != _downloadUrls[i]) { shouldUpdateUrls = true; break; } @@ -215,10 +214,8 @@ void Stats::updateStats(bool force) { // If the urls have changed, update the list if (shouldUpdateUrls) { _downloadUrls.clear(); - foreach (QSharedPointer request, loadingRequests) { - if (request) { - _downloadUrls << request->getURL().toString(); - } + foreach (auto resource, loadingRequests) { + _downloadUrls << resource->getURL().toString(); } emit downloadUrlsChanged(); } diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index 11d0f31217..6c74da749c 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -41,9 +41,9 @@ void ResourceCache::refreshAll() { resetResourceCounters(); // Refresh all remaining resources in use - foreach (auto resource, _resources) { - if (!resource.isNull()) { - resource.data()->refresh(); + foreach (QSharedPointer resource, _resources) { + if (resource) { + resource->refresh(); } } } @@ -110,7 +110,7 @@ QSharedPointer ResourceCache::getResource(const QUrl& url, const QUrl& } if (QThread::currentThread() != thread()) { - assert(delayLoad && !extra); + assert(delayLoad); getResourceAsynchronously(url); return QSharedPointer(); } @@ -208,9 +208,19 @@ void ResourceCacheSharedItems::appendPendingRequest(QWeakPointer resou _pendingRequests.append(resource); } -QList> ResourceCacheSharedItems::getPendingRequests() const { - Lock lock(_mutex); - return _pendingRequests; +QList> ResourceCacheSharedItems::getPendingRequests() { + QList> result; + + { + Lock lock(_mutex); + foreach(QSharedPointer resource, _pendingRequests) { + if (resource) { + result.append(resource); + } + } + _pendingRequests.removeAll(QWeakPointer()); + } + return result; } uint32_t ResourceCacheSharedItems::getPendingRequestsCount() const { @@ -218,9 +228,19 @@ uint32_t ResourceCacheSharedItems::getPendingRequestsCount() const { return _pendingRequests.size(); } -QList> ResourceCacheSharedItems::getLoadingRequests() const { - Lock lock(_mutex); - return _loadingRequests; +QList> ResourceCacheSharedItems::getLoadingRequests() { + QList> result; + + { + Lock lock(_mutex); + foreach(QSharedPointer resource, _loadingRequests) { + if (resource) { + result.append(resource); + } + } + _loadingRequests.removeAll(QWeakPointer()); + } + return result; } void ResourceCacheSharedItems::removeRequest(QWeakPointer resource) { @@ -254,7 +274,7 @@ QSharedPointer ResourceCacheSharedItems::getHighestPendingRequest() { return highestResource; } -const QList> ResourceCache::getLoadingRequests() { +QList> ResourceCache::getLoadingRequests() { return DependencyManager::get()->getLoadingRequests(); } diff --git a/libraries/networking/src/ResourceCache.h b/libraries/networking/src/ResourceCache.h index 034ca74882..94fcafee9e 100644 --- a/libraries/networking/src/ResourceCache.h +++ b/libraries/networking/src/ResourceCache.h @@ -65,9 +65,9 @@ public: void appendPendingRequest(QWeakPointer newRequest); void appendActiveRequest(QWeakPointer newRequest); void removeRequest(QWeakPointer doneRequest); - QList> getPendingRequests() const; + QList> getPendingRequests(); uint32_t getPendingRequestsCount() const; - QList> getLoadingRequests() const; + QList> getLoadingRequests(); QSharedPointer getHighestPendingRequest(); private: @@ -88,8 +88,6 @@ class ResourceCache : public QObject { Q_PROPERTY(size_t sizeCached READ getSizeCachedResources NOTIFY dirty) public: - virtual ~ResourceCache(); - size_t getNumTotalResources() const { return _numTotalResources; } size_t getSizeTotalResources() const { return _totalResourcesSize; } @@ -106,10 +104,12 @@ public: void setUnusedResourceCacheSize(qint64 unusedResourcesMaxSize); qint64 getUnusedResourceCacheSize() const { return _unusedResourcesMaxSize; } - static const QList> getLoadingRequests(); + static QList> getLoadingRequests(); static int getPendingRequestCount(); + ResourceCache(QObject* parent = nullptr); + virtual ~ResourceCache(); void refreshAll(); void refresh(const QUrl& url); @@ -124,8 +124,6 @@ protected slots: void updateTotalSize(const qint64& oldSize, const qint64& newSize); protected: - ResourceCache(QObject* parent = nullptr); - /// Loads a resource from the specified URL. /// \param fallback a fallback URL to load if the desired one is unavailable /// \param delayLoad if true, don't load the resource immediately; wait until load is first requested From 5cc90ce8f0296c83abe41108c9b7e295ac01fe11 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Mon, 11 Apr 2016 11:22:53 -0700 Subject: [PATCH 10/40] Make sure resource req are cleaned up with there parents --- libraries/networking/src/ResourceManager.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/networking/src/ResourceManager.cpp b/libraries/networking/src/ResourceManager.cpp index 8fc8e160b0..f33dbc161d 100644 --- a/libraries/networking/src/ResourceManager.cpp +++ b/libraries/networking/src/ResourceManager.cpp @@ -110,6 +110,9 @@ ResourceRequest* ResourceManager::createResourceRequest(QObject* parent, const Q } Q_ASSERT(request); + if (parent) { + QObject::connect(parent, &QObject::destroyed, request, &QObject::deleteLater); + } request->moveToThread(&_thread); return request; } From 05895f628ac7bcccf8ebc3376e2644a92993b566 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Tue, 12 Apr 2016 13:34:33 -0700 Subject: [PATCH 11/40] Revert to using a Resource --- libraries/animation/src/AnimNodeLoader.cpp | 20 ++++++-------------- libraries/animation/src/AnimNodeLoader.h | 9 +++++---- libraries/networking/src/ResourceCache.cpp | 2 -- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/libraries/animation/src/AnimNodeLoader.cpp b/libraries/animation/src/AnimNodeLoader.cpp index f5d9215253..df39c55e5d 100644 --- a/libraries/animation/src/AnimNodeLoader.cpp +++ b/libraries/animation/src/AnimNodeLoader.cpp @@ -570,19 +570,11 @@ bool processStateMachineNode(AnimNode::Pointer node, const QJsonObject& jsonObj, AnimNodeLoader::AnimNodeLoader(const QUrl& url) : _url(url) { - auto request = ResourceManager::createResourceRequest(this, url); - if (request) { - connect(request, &ResourceRequest::finished, this, [this, request]() { - if (request->getResult() == ResourceRequest::Success) { - onRequestDone(request->getData()); - } else { - onRequestError(request->getResult()); - } - request->deleteLater(); - }); - - request->send(); - } + _resource = QSharedPointer::create(url); + _resource->setSelf(_resource); + connect(_resource.data(), &Resource::loaded, this, &AnimNodeLoader::onRequestDone); + connect(_resource.data(), &Resource::failed, this, &AnimNodeLoader::onRequestError); + _resource->ensureLoading(); } AnimNode::Pointer AnimNodeLoader::load(const QByteArray& contents, const QUrl& jsonUrl) { @@ -629,6 +621,6 @@ void AnimNodeLoader::onRequestDone(const QByteArray data) { } } -void AnimNodeLoader::onRequestError(ResourceRequest::Result netError) { +void AnimNodeLoader::onRequestError(QNetworkReply::NetworkError netError) { emit error((int)netError, "Resource download error"); } diff --git a/libraries/animation/src/AnimNodeLoader.h b/libraries/animation/src/AnimNodeLoader.h index edb4729753..6db001c261 100644 --- a/libraries/animation/src/AnimNodeLoader.h +++ b/libraries/animation/src/AnimNodeLoader.h @@ -13,13 +13,13 @@ #include +#include #include #include #include "AnimNode.h" -#include - +class Resource; class AnimNodeLoader : public QObject { Q_OBJECT @@ -37,11 +37,12 @@ protected: protected slots: void onRequestDone(const QByteArray data); - void onRequestError(ResourceRequest::Result error); + void onRequestError(QNetworkReply::NetworkError error); protected: QUrl _url; - + QSharedPointer _resource; + private: // no copies diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index 6c74da749c..2f90f36670 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -218,7 +218,6 @@ QList> ResourceCacheSharedItems::getPendingRequests() { result.append(resource); } } - _pendingRequests.removeAll(QWeakPointer()); } return result; } @@ -238,7 +237,6 @@ QList> ResourceCacheSharedItems::getLoadingRequests() { result.append(resource); } } - _loadingRequests.removeAll(QWeakPointer()); } return result; } From c8aeecdabdacefa902528d2f8c3f2bb298f1f575 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Tue, 12 Apr 2016 13:41:22 -0700 Subject: [PATCH 12/40] Don't fire onGeoMappLoading twice --- .../src/model-networking/ModelCache.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/libraries/model-networking/src/model-networking/ModelCache.cpp b/libraries/model-networking/src/model-networking/ModelCache.cpp index b8deef9f27..387c59e7f4 100644 --- a/libraries/model-networking/src/model-networking/ModelCache.cpp +++ b/libraries/model-networking/src/model-networking/ModelCache.cpp @@ -46,6 +46,7 @@ private slots: private: GeometryResource::Pointer _geometryResource; + QMetaObject::Connection _connection; }; void GeometryMappingResource::downloadFinished(const QByteArray& data) { @@ -77,21 +78,26 @@ void GeometryMappingResource::downloadFinished(const QByteArray& data) { if (_geometryResource->isLoaded()) { onGeometryMappingLoaded(!_geometryResource->getURL().isEmpty()); } else { - connect(_geometryResource.data(), &Resource::finished, this, &GeometryMappingResource::onGeometryMappingLoaded); + if (_connection) { + disconnect(_connection); + } + + _connection = connect(_geometryResource.data(), &Resource::finished, + this, &GeometryMappingResource::onGeometryMappingLoaded); } } } void GeometryMappingResource::onGeometryMappingLoaded(bool success) { - if (success) { + if (success && _geometryResource) { _geometry = _geometryResource->_geometry; _shapes = _geometryResource->_shapes; _meshes = _geometryResource->_meshes; _materials = _geometryResource->_materials; - } - // Avoid holding onto extra references - _geometryResource.reset(); + // Avoid holding onto extra references + _geometryResource.reset(); + } finishedLoading(success); } From 364f935b893505afd6873816ecf2815ad7bb41dd Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Tue, 12 Apr 2016 13:45:12 -0700 Subject: [PATCH 13/40] Allow cleanup of the old request --- libraries/networking/src/ResourceCache.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index 2f90f36670..5f7cf7926a 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -450,7 +450,10 @@ void Resource::reinsert() { void Resource::makeRequest() { - Q_ASSERT(!_request); + if (_request) { + _request->disconnect(); + _request->deleteLater(); + } _request = ResourceManager::createResourceRequest(this, _activeUrl); From 88150f9c82b7c0e296be5d9cd99888b15f3485ab Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 12 Apr 2016 14:45:05 -0700 Subject: [PATCH 14/40] Remove CrashHelpers --- libraries/shared/src/CrashHelpers.h | 80 ----------------------------- 1 file changed, 80 deletions(-) delete mode 100644 libraries/shared/src/CrashHelpers.h diff --git a/libraries/shared/src/CrashHelpers.h b/libraries/shared/src/CrashHelpers.h deleted file mode 100644 index 9146df746c..0000000000 --- a/libraries/shared/src/CrashHelpers.h +++ /dev/null @@ -1,80 +0,0 @@ -// -// CrashHelpers.h -// libraries/shared/src -// -// Created by Ryan Huffman on 11 April 2016. -// Copyright 2016 High Fidelity, Inc. -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#pragma once - -#ifndef hifi_CrashHelpers_h -#define hifi_CrashHelpers_h - -namespace crash { - -class B; -class A { -public: - A(B* b) : _b(b) { } - ~A(); - virtual void virtualFunction() = 0; - -private: - B* _b; -}; - -class B : public A { -public: - B() : A(this) { } - virtual void virtualFunction() { } -}; - -A::~A() { - _b->virtualFunction(); -} - -void pureVirtualCall() { - - qDebug() << "About to make a pure virtual call"; - { - B b; - } -} - -void doubleFree() { - qDebug() << "About to double delete memory"; - int* blah = new int(200); - delete blah; - delete blah; -} - -void nullDeref() { - qDebug() << "About to dereference a null pointer"; - int* p = nullptr; - *p = 1; -} - -void doAbort() { - qDebug() << "About to abort"; - abort(); -} - -void outOfBoundsVectorCrash() { - qDebug() << "std::vector out of bounds crash!"; - std::vector v; - v[0] = 42; -} - -void newFault() { - qDebug() << "About to crash inside new fault"; - // Force crash with large allocation - int *crash = new int[std::numeric_limits::max()]; -} - -} - -#endif // hifi_CrashHelpers_h From 7c652aa3cad686e853b3430d4104fd3c9da0da62 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Mon, 11 Apr 2016 10:07:55 -0700 Subject: [PATCH 15/40] Add lock on ResourceCache data structs --- libraries/networking/src/ResourceCache.cpp | 43 +++++++++++++++++----- libraries/networking/src/ResourceCache.h | 4 +- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index 2273902263..0bd3b5ab34 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -39,20 +39,30 @@ void ResourceCache::refreshAll() { // Clear all unused resources so we don't have to reload them clearUnusedResource(); resetResourceCounters(); - + + _resourcesLock.lockForRead(); + auto resourcesCopy = _resources; + _resourcesLock.unlock(); + // Refresh all remaining resources in use - foreach (auto resource, _resources) { - if (!resource.isNull()) { - resource.data()->refresh(); + foreach (QSharedPointer resource, resourcesCopy) { + if (resource) { + resource->refresh(); } } } void ResourceCache::refresh(const QUrl& url) { - QSharedPointer resource = _resources.value(url); - if (!resource.isNull()) { + QSharedPointer resource; + { + QReadLocker locker(&_resourcesLock); + resource = _resources.value(url).lock(); + } + + if (resource) { resource->refresh(); } else { + QWriteLocker locker(&_resourcesLock); _resources.remove(url); resetResourceCounters(); } @@ -103,8 +113,12 @@ void ResourceCache::checkAsynchronousGets() { QSharedPointer ResourceCache::getResource(const QUrl& url, const QUrl& fallback, bool delayLoad, void* extra) { - QSharedPointer resource = _resources.value(url); - if (!resource.isNull()) { + QSharedPointer resource; + { + QReadLocker locker(&_resourcesLock); + resource = _resources.value(url).lock(); + } + if (resource) { removeUnusedResource(resource); return resource; } @@ -123,7 +137,10 @@ QSharedPointer ResourceCache::getResource(const QUrl& url, const QUrl& getResource(fallback, QUrl(), true) : QSharedPointer(), delayLoad, extra); resource->setSelf(resource); resource->setCache(this); - _resources.insert(url, resource); + { + QWriteLocker locker(&_resourcesLock); + _resources.insert(url, resource); + } removeUnusedResource(resource); resource->ensureLoading(); @@ -145,13 +162,16 @@ void ResourceCache::addUnusedResource(const QSharedPointer& resource) reserveUnusedResource(resource->getBytes()); resource->setLRUKey(++_lastLRUKey); - _unusedResources.insert(resource->getLRUKey(), resource); _unusedResourcesSize += resource->getBytes(); resetResourceCounters(); + + QWriteLocker locker(&_unusedResourcesLock); + _unusedResources.insert(resource->getLRUKey(), resource); } void ResourceCache::removeUnusedResource(const QSharedPointer& resource) { + QWriteLocker locker(&_unusedResourcesLock); if (_unusedResources.contains(resource->getLRUKey())) { _unusedResources.remove(resource->getLRUKey()); _unusedResourcesSize -= resource->getBytes(); @@ -160,6 +180,7 @@ void ResourceCache::removeUnusedResource(const QSharedPointer& resourc } void ResourceCache::reserveUnusedResource(qint64 resourceSize) { + QWriteLocker locker(&_unusedResourcesLock); while (!_unusedResources.empty() && _unusedResourcesSize + resourceSize > _unusedResourcesMaxSize) { // unload the oldest resource @@ -179,6 +200,7 @@ void ResourceCache::reserveUnusedResource(qint64 resourceSize) { void ResourceCache::clearUnusedResource() { // the unused resources may themselves reference resources that will be added to the unused // list on destruction, so keep clearing until there are no references left + QWriteLocker locker(&_unusedResourcesLock); while (!_unusedResources.isEmpty()) { foreach (const QSharedPointer& resource, _unusedResources) { resource->setCache(nullptr); @@ -422,6 +444,7 @@ void Resource::setSize(const qint64& bytes) { } void Resource::reinsert() { + QWriteLocker locker(&_cache->_resourcesLock); _cache->_resources.insert(_url, _self); } diff --git a/libraries/networking/src/ResourceCache.h b/libraries/networking/src/ResourceCache.h index 84eba1cdc0..677c7d81a3 100644 --- a/libraries/networking/src/ResourceCache.h +++ b/libraries/networking/src/ResourceCache.h @@ -154,6 +154,7 @@ private: void clearUnusedResource(); void resetResourceCounters(); + QReadWriteLock _resourcesLock { QReadWriteLock::Recursive }; QHash> _resources; int _lastLRUKey = 0; @@ -161,7 +162,7 @@ private: static int _requestsActive; void getResourceAsynchronously(const QUrl& url); - QReadWriteLock _resourcesToBeGottenLock; + QReadWriteLock _resourcesToBeGottenLock { QReadWriteLock::Recursive }; QQueue _resourcesToBeGotten; std::atomic _numTotalResources { 0 }; @@ -171,6 +172,7 @@ private: std::atomic _unusedResourcesSize { 0 }; qint64 _unusedResourcesMaxSize = DEFAULT_UNUSED_MAX_SIZE; + QReadWriteLock _unusedResourcesLock { QReadWriteLock::Recursive }; QMap> _unusedResources; }; From 0f62d5c9979d4718ae6637f503a3021ba150f3fa Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Mon, 11 Apr 2016 10:23:30 -0700 Subject: [PATCH 16/40] clear ATP assets on domain change --- libraries/networking/src/ResourceCache.cpp | 43 +++++++++++++++++++++- libraries/networking/src/ResourceCache.h | 3 ++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index 0bd3b5ab34..a208d39402 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -20,6 +20,7 @@ #include "NetworkAccessManager.h" #include "NetworkLogging.h" +#include "NodeList.h" #include "ResourceCache.h" @@ -27,14 +28,52 @@ (((x) > (max)) ? (max) :\ (x))) -ResourceCache::ResourceCache(QObject* parent) : - QObject(parent) { +ResourceCache::ResourceCache(QObject* parent) : QObject(parent) { + auto& domainHandler = DependencyManager::get()->getDomainHandler(); + connect(&domainHandler, &DomainHandler::disconnectedFromDomain, + this, &ResourceCache::clearATPAssets, Qt::DirectConnection); } ResourceCache::~ResourceCache() { clearUnusedResource(); } +void ResourceCache::clearATPAssets() { + { + QWriteLocker locker(&_resourcesLock); + for (auto& url : _resources.keys()) { + // If this is an ATP resource + if (url.scheme() == URL_SCHEME_ATP) { + + // Remove it from the resource hash + auto resource = _resources.take(url); + if (auto strongRef = resource.lock()) { + // Make sure the resource won't reinsert itself + strongRef->setCache(nullptr); + } + } + } + } + { + QWriteLocker locker(&_unusedResourcesLock); + for (auto& resource : _unusedResources.values()) { + if (resource->getURL().scheme() == URL_SCHEME_ATP) { + _unusedResources.remove(resource->getLRUKey()); + } + } + } + { + QWriteLocker locker(&_resourcesToBeGottenLock); + for (auto& url : _resourcesToBeGotten) { + if (url.scheme() == URL_SCHEME_ATP) { + _resourcesToBeGotten.removeAll(url); + } + } + } + + +} + void ResourceCache::refreshAll() { // Clear all unused resources so we don't have to reload them clearUnusedResource(); diff --git a/libraries/networking/src/ResourceCache.h b/libraries/networking/src/ResourceCache.h index 677c7d81a3..5fb82415b0 100644 --- a/libraries/networking/src/ResourceCache.h +++ b/libraries/networking/src/ResourceCache.h @@ -126,6 +126,9 @@ public slots: protected slots: void updateTotalSize(const qint64& oldSize, const qint64& newSize); +private slots: + void clearATPAssets(); + protected: /// Loads a resource from the specified URL. /// \param fallback a fallback URL to load if the desired one is unavailable From b5fe6120aa996a9828bdc22108d199d4a9851bde Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 12 Apr 2016 17:41:47 -0700 Subject: [PATCH 17/40] base randomization of ice-server from ice.highfidelity.com --- domain-server/src/DomainServer.cpp | 97 +++++++++++++++++++++++++++++- domain-server/src/DomainServer.h | 15 ++++- 2 files changed, 108 insertions(+), 4 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 16928f3dee..40f0550568 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -12,6 +12,7 @@ #include "DomainServer.h" #include +#include #include #include @@ -42,7 +43,7 @@ int const DomainServer::EXIT_CODE_REBOOT = 234923; -const QString ICE_SERVER_DEFAULT_HOSTNAME = "ice.highfidelity.io"; +const QString ICE_SERVER_DEFAULT_HOSTNAME = "ice.highfidelity.com"; DomainServer::DomainServer(int argc, char* argv[]) : QCoreApplication(argc, argv), @@ -59,8 +60,7 @@ DomainServer::DomainServer(int argc, char* argv[]) : _webAuthenticationStateSet(), _cookieSessionHash(), _automaticNetworkingSetting(), - _settingsManager(), - _iceServerSocket(ICE_SERVER_DEFAULT_HOSTNAME, ICE_SERVER_DEFAULT_PORT) + _settingsManager() { qInstallMessageHandler(LogHandler::verboseMessageHandler); @@ -482,6 +482,18 @@ void DomainServer::setupAutomaticNetworking() { void DomainServer::setupICEHeartbeatForFullNetworking() { auto limitedNodeList = DependencyManager::get(); + // lookup the available ice-server hosts now + updateICEServerAddresses(); + + const int ICE_ADDRESS_UPDATE_MSECS = 30 * 1000; + + // hookup a timer to keep those updated every ICE_ADDRESS_UPDATE_MSECS in case of a failure requiring a switchover + if (_iceAddressLookupTimer) { + _iceAddressLookupTimer = new QTimer { this }; + connect(_iceAddressLookupTimer, &QTimer::timeout, this, &DomainServer::updateICEServerAddresses); + _iceAddressLookupTimer->start(ICE_ADDRESS_UPDATE_MSECS); + } + // call our sendHeartbeatToIceServer immediately anytime a local or public socket changes connect(limitedNodeList.data(), &LimitedNodeList::localSockAddrChanged, this, &DomainServer::sendHeartbeatToIceServer); @@ -512,6 +524,12 @@ void DomainServer::setupICEHeartbeatForFullNetworking() { } } +void DomainServer::updateICEServerAddresses() { + if (_iceAddressLookupID == -1) { + _iceAddressLookupID = QHostInfo::lookupHost(ICE_SERVER_DEFAULT_HOSTNAME, this, SLOT(handleICEHostInfo(QHostInfo))); + } +} + void DomainServer::parseAssignmentConfigs(QSet& excludedTypes) { // check for configs from the command line, these take precedence const QString ASSIGNMENT_CONFIG_REGEX_STRING = "config-([\\d]+)"; @@ -1135,6 +1153,10 @@ void DomainServer::sendHeartbeatToIceServer() { // send the heartbeat packet to the ice server now limitedNodeList->sendUnreliablePacket(*_iceServerHeartbeatPacket, _iceServerSocket); + } else { + qDebug() << "Not sending ice-server heartbeat since there is no selected ice-server."; + qDebug() << "Waiting for" << ICE_SERVER_DEFAULT_HOSTNAME << "host lookup response"; + } } @@ -2033,3 +2055,72 @@ void DomainServer::handleKeypairChange() { sendHeartbeatToIceServer(); } } + +void DomainServer::handleICEHostInfo(const QHostInfo& hostInfo) { + // clear the ICE address lookup ID so that it can fire again + _iceAddressLookupID = -1; + + if (hostInfo.error() != QHostInfo::NoError) { + qWarning() << "IP address lookup failed for" << ICE_SERVER_DEFAULT_HOSTNAME << ":" << hostInfo.errorString(); + + // if we don't have an ICE server to use yet, trigger a retry + if (_iceServerSocket.isNull()) { + const int ICE_ADDRESS_LOOKUP_RETRY_MS = 1000; + + QTimer::singleShot(ICE_ADDRESS_LOOKUP_RETRY_MS, this, SLOT(updateICEServerAddresses())); + } + + } else { + int countBefore = _iceServerAddresses.count(); + + _iceServerAddresses = hostInfo.addresses(); + + if (countBefore == 0) { + qInfo() << "Found" << _iceServerAddresses.count() << "ice-server IP addresses for" << ICE_SERVER_DEFAULT_HOSTNAME; + } + + if (_iceServerSocket.isNull()) { + // we don't have a candidate ice-server yet, pick now + randomizeICEServerAddress(); + } + } +} + +void DomainServer::randomizeICEServerAddress() { + // create a list by removing the already failed ice-server addresses + auto candidateICEAddresses = _iceServerAddresses; + + auto it = candidateICEAddresses.begin(); + + while (it != candidateICEAddresses.end()) { + if (_failedIceServerAddresses.contains(*it)) { + // we already tried this address and it failed, remove it from list of candidates + it = candidateICEAddresses.erase(it); + } else { + // keep this candidate, it hasn't failed yet + ++it; + } + } + + if (candidateICEAddresses.empty()) { + // we ended up with an empty list since everything we've tried has failed + // so clear the set of failed addresses and start going through them again + _failedIceServerAddresses.clear(); + candidateICEAddresses = _iceServerAddresses; + } + + // of the list of available addresses that we haven't tried, pick a random one + int maxIndex = candidateICEAddresses.size(); + + static std::random_device randomDevice; + static std::mt19937 generator; + std::uniform_int_distribution<> distribution(0, maxIndex); + + auto indexToTry = distribution(generator); + + _iceServerSocket = HifiSockAddr { candidateICEAddresses[indexToTry], ICE_SERVER_DEFAULT_PORT }; + qInfo() << "Set candidate ice-server socket to" << _iceServerSocket; + + // immediately fire an ICE heartbeat once we've picked a candidate ice-server + sendHeartbeatToIceServer(); +} diff --git a/domain-server/src/DomainServer.h b/domain-server/src/DomainServer.h index 3a83e8696b..6decbc29d0 100644 --- a/domain-server/src/DomainServer.h +++ b/domain-server/src/DomainServer.h @@ -81,6 +81,12 @@ private slots: void queuedQuit(QString quitMessage, int exitCode); void handleKeypairChange(); + + void updateICEServerAddresses(); + void handleICEHostInfo(const QHostInfo& hostInfo); + +signals: + void iceServerChanged(); private: void setupNodeListAndAssignments(const QUuid& sessionUUID = QUuid::createUuid()); @@ -95,6 +101,8 @@ private: void setupICEHeartbeatForFullNetworking(); void sendHeartbeatToDataServer(const QString& networkAddress); + void randomizeICEServerAddress(); + unsigned int countConnectedUsers(); void sendDomainListToNode(const SharedNodePointer& node, const HifiSockAddr& senderSockAddr); @@ -157,7 +165,12 @@ private: std::unique_ptr _iceServerHeartbeatPacket; QTimer* _iceHeartbeatTimer { nullptr }; // this looks like it dangles when created but it's parented to the DomainServer - + + QList _iceServerAddresses; + QSet _failedIceServerAddresses; + QTimer* _iceAddressLookupTimer { nullptr }; // this looks like a dangling pointer but is parented to the DomainServer + int _iceAddressLookupID { -1 }; + friend class DomainGatekeeper; }; From f975f480b486ab498626abae3e42a7df12ba4074 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 12 Apr 2016 17:46:47 -0700 Subject: [PATCH 18/40] use the random device for random number generation --- domain-server/src/DomainServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 40f0550568..327afdf292 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -2113,7 +2113,7 @@ void DomainServer::randomizeICEServerAddress() { int maxIndex = candidateICEAddresses.size(); static std::random_device randomDevice; - static std::mt19937 generator; + static std::mt19937 generator(randomDevice()); std::uniform_int_distribution<> distribution(0, maxIndex); auto indexToTry = distribution(generator); From 8832a9d9ed6111050c99474fa02a11c86b7cb3a9 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 12 Apr 2016 17:59:07 -0700 Subject: [PATCH 19/40] add ICEServerHeartbeatACK packet type --- libraries/networking/src/udt/PacketHeaders.cpp | 4 ++-- libraries/networking/src/udt/PacketHeaders.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index df1a6f1bec..e4aab94090 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -34,8 +34,8 @@ const QSet NON_SOURCED_PACKETS = QSet() << PacketType::DomainServerAddedNode << PacketType::DomainServerConnectionToken << PacketType::DomainSettingsRequest << PacketType::DomainSettings << PacketType::ICEServerPeerInformation << PacketType::ICEServerQuery << PacketType::ICEServerHeartbeat - << PacketType::ICEPing << PacketType::ICEPingReply << PacketType::ICEServerHeartbeatDenied - << PacketType::AssignmentClientStatus << PacketType::StopNode + << PacketType::ICEServerHeartbeatACK << PacketType::ICEPing << PacketType::ICEPingReply + << PacketType::ICEServerHeartbeatDenied << PacketType::AssignmentClientStatus << PacketType::StopNode << PacketType::DomainServerRemovedNode; const QSet RELIABLE_PACKETS = QSet(); diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index a2bd0d4d2d..b98a87e439 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -93,7 +93,8 @@ public: MessagesUnsubscribe, ICEServerHeartbeatDenied, AssetMappingOperation, - AssetMappingOperationReply + AssetMappingOperationReply, + ICEServerHeartbeatACK }; }; From 6ef9fbfcc09b2b98446f3d78c171bb0a54e62e2a Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 12 Apr 2016 18:01:20 -0700 Subject: [PATCH 20/40] send an ACK packet from ice server for verified hearbeat --- ice-server/src/IceServer.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ice-server/src/IceServer.cpp b/ice-server/src/IceServer.cpp index b66ccaa057..c3e57b1d36 100644 --- a/ice-server/src/IceServer.cpp +++ b/ice-server/src/IceServer.cpp @@ -82,6 +82,11 @@ void IceServer::processPacket(std::unique_ptr packet) { if (peer) { // so that we can send packets to the heartbeating peer when we need, we need to activate a socket now peer->activateMatchingOrNewSymmetricSocket(nlPacket->getSenderSockAddr()); + + // we have an active and verified heartbeating peer + // send them an ACK packet so they know that they are being heard and ready for ICE + static auto ackPacket = NLPacket::create(PacketType::ICEServerHeartbeatACK); + _serverSocket.writePacket(*ackPacket, nlPacket->getSenderSockAddr()); } else { // we couldn't verify this peer - respond back to them so they know they may need to perform keypair re-generation static auto deniedPacket = NLPacket::create(PacketType::ICEServerHeartbeatDenied); From f1209dc82d5eb8a2791893c7edcd5b85dfbed14c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 12 Apr 2016 18:04:32 -0700 Subject: [PATCH 21/40] fix index randomization for single candidate ice-server --- domain-server/src/DomainServer.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 327afdf292..4dd7fefd2a 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -2110,13 +2110,16 @@ void DomainServer::randomizeICEServerAddress() { } // of the list of available addresses that we haven't tried, pick a random one - int maxIndex = candidateICEAddresses.size(); + int maxIndex = candidateICEAddresses.size() - 1; + int indexToTry = 0; - static std::random_device randomDevice; - static std::mt19937 generator(randomDevice()); - std::uniform_int_distribution<> distribution(0, maxIndex); + if (maxIndex > 0) { + static std::random_device randomDevice; + static std::mt19937 generator(randomDevice()); + std::uniform_int_distribution<> distribution(0, maxIndex); - auto indexToTry = distribution(generator); + indexToTry = distribution(generator); + } _iceServerSocket = HifiSockAddr { candidateICEAddresses[indexToTry], ICE_SERVER_DEFAULT_PORT }; qInfo() << "Set candidate ice-server socket to" << _iceServerSocket; From f69f59fa90b07bf30e84d957f120b910571e5112 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 12 Apr 2016 18:19:02 -0700 Subject: [PATCH 22/40] handle ice fail for no reply heartbeat in DS --- domain-server/src/DomainServer.cpp | 34 ++++++++++++++++++++++++++++++ domain-server/src/DomainServer.h | 2 ++ 2 files changed, 36 insertions(+) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 4dd7fefd2a..1ba2691dbc 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -369,7 +369,9 @@ void DomainServer::setupNodeListAndAssignments(const QUuid& sessionUUID) { packetReceiver.registerListener(PacketType::ICEPing, &_gatekeeper, "processICEPingPacket"); packetReceiver.registerListener(PacketType::ICEPingReply, &_gatekeeper, "processICEPingReplyPacket"); packetReceiver.registerListener(PacketType::ICEServerPeerInformation, &_gatekeeper, "processICEPeerInformationPacket"); + packetReceiver.registerListener(PacketType::ICEServerHeartbeatDenied, this, "processICEServerHeartbeatDenialPacket"); + packetReceiver.registerListener(PacketType::ICEServerHeartbeatACK, this, "processICEServerHeartbeatACK"); // add whatever static assignments that have been parsed to the queue addStaticAssignmentsToQueue(); @@ -1102,6 +1104,25 @@ void DomainServer::sendHeartbeatToIceServer() { return; } + const int FAILOVER_NO_REPLY_ICE_HEARTBEATS { 3 }; + + // increase the count of no reply ICE heartbeats and check the current value + ++_noReplyICEHeartbeats; + + if (_noReplyICEHeartbeats > FAILOVER_NO_REPLY_ICE_HEARTBEATS) { + qWarning() << "There have been" << _noReplyICEHeartbeats - 1 << "heartbeats sent with no reply from the ice-server"; + qWarning() << "Clearing the current ice-server socket and selecting a new candidate ice-server"; + + // if we've failed to hear back for three heartbeats, we clear the current ice-server socket and attempt + // to randomize a new one + _iceServerSocket.clear(); + + // reset the number of no reply ICE hearbeats + _noReplyICEHeartbeats = 0; + + randomizeICEServerAddress(); + } + // NOTE: I'd love to specify the correct size for the packet here, but it's a little trickey with // QDataStream and the possibility of IPv6 address for the sockets. if (!_iceServerHeartbeatPacket) { @@ -1153,6 +1174,7 @@ void DomainServer::sendHeartbeatToIceServer() { // send the heartbeat packet to the ice server now limitedNodeList->sendUnreliablePacket(*_iceServerHeartbeatPacket, _iceServerSocket); + } else { qDebug() << "Not sending ice-server heartbeat since there is no selected ice-server."; qDebug() << "Waiting for" << ICE_SERVER_DEFAULT_HOSTNAME << "host lookup response"; @@ -2043,6 +2065,14 @@ void DomainServer::processICEServerHeartbeatDenialPacket(QSharedPointer message) { + // we don't do anything with this ACK other than use it to tell us to keep talking to the same ice-server + _noReplyICEHeartbeats = 0; } void DomainServer::handleKeypairChange() { @@ -2105,6 +2135,10 @@ void DomainServer::randomizeICEServerAddress() { if (candidateICEAddresses.empty()) { // we ended up with an empty list since everything we've tried has failed // so clear the set of failed addresses and start going through them again + + qWarning() << "All current ice-server addresses have failed - re-attempting all current addresses for" + << ICE_SERVER_DEFAULT_HOSTNAME; + _failedIceServerAddresses.clear(); candidateICEAddresses = _iceServerAddresses; } diff --git a/domain-server/src/DomainServer.h b/domain-server/src/DomainServer.h index 6decbc29d0..7fa83a7395 100644 --- a/domain-server/src/DomainServer.h +++ b/domain-server/src/DomainServer.h @@ -62,6 +62,7 @@ public slots: void processPathQueryPacket(QSharedPointer packet); void processNodeDisconnectRequestPacket(QSharedPointer message); void processICEServerHeartbeatDenialPacket(QSharedPointer message); + void processICEServerHeartbeatACK(QSharedPointer message); private slots: void aboutToQuit(); @@ -170,6 +171,7 @@ private: QSet _failedIceServerAddresses; QTimer* _iceAddressLookupTimer { nullptr }; // this looks like a dangling pointer but is parented to the DomainServer int _iceAddressLookupID { -1 }; + int _noReplyICEHeartbeats { 0 }; friend class DomainGatekeeper; }; From ab414f65eb8951f83cdae87a632528308fd69f05 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 12 Apr 2016 18:23:51 -0700 Subject: [PATCH 23/40] output success on first connection to new ice-server --- domain-server/src/DomainServer.cpp | 8 ++++++++ domain-server/src/DomainServer.h | 1 + 2 files changed, 9 insertions(+) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 1ba2691dbc..3b2f13c668 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -1120,6 +1120,9 @@ void DomainServer::sendHeartbeatToIceServer() { // reset the number of no reply ICE hearbeats _noReplyICEHeartbeats = 0; + // reset the connection flag for ICE server + _connectedToICEServer = false; + randomizeICEServerAddress(); } @@ -2073,6 +2076,11 @@ void DomainServer::processICEServerHeartbeatDenialPacket(QSharedPointer message) { // we don't do anything with this ACK other than use it to tell us to keep talking to the same ice-server _noReplyICEHeartbeats = 0; + + if (!_connectedToICEServer) { + _connectedToICEServer = true; + qInfo() << "Connected to ice-server at" << _iceServerSocket; + } } void DomainServer::handleKeypairChange() { diff --git a/domain-server/src/DomainServer.h b/domain-server/src/DomainServer.h index 7fa83a7395..1165c76559 100644 --- a/domain-server/src/DomainServer.h +++ b/domain-server/src/DomainServer.h @@ -172,6 +172,7 @@ private: QTimer* _iceAddressLookupTimer { nullptr }; // this looks like a dangling pointer but is parented to the DomainServer int _iceAddressLookupID { -1 }; int _noReplyICEHeartbeats { 0 }; + bool _connectedToICEServer { false }; friend class DomainGatekeeper; }; From 2d39551a35570886a6a49385dcc1341de8aa3fc1 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 13 Apr 2016 10:56:27 -0700 Subject: [PATCH 24/40] Fix style in CrashReporter.cpp --- interface/src/CrashReporter.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/interface/src/CrashReporter.cpp b/interface/src/CrashReporter.cpp index de8782e279..1fd534ffac 100644 --- a/interface/src/CrashReporter.cpp +++ b/interface/src/CrashReporter.cpp @@ -99,16 +99,11 @@ int handleNewError(size_t size) { throw("ERROR: Errors calling new"); } -LPTOP_LEVEL_EXCEPTION_FILTER WINAPI -noop_SetUnhandledExceptionFilter( - LPTOP_LEVEL_EXCEPTION_FILTER lpTopLevelExceptionFilter) -{ +LPTOP_LEVEL_EXCEPTION_FILTER WINAPI noop_SetUnhandledExceptionFilter(LPTOP_LEVEL_EXCEPTION_FILTER lpTopLevelExceptionFilter) { return nullptr; } -_purecall_handler __cdecl -noop_set_purecall_handler(_purecall_handler pNew) -{ +_purecall_handler __cdecl noop_set_purecall_handler(_purecall_handler pNew) { return nullptr; } From 785eda44cd8df122f9ba37aae2058065b2aeb97c Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Wed, 13 Apr 2016 13:23:11 -0700 Subject: [PATCH 25/40] CR --- libraries/model-networking/src/model-networking/ModelCache.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/model-networking/src/model-networking/ModelCache.cpp b/libraries/model-networking/src/model-networking/ModelCache.cpp index 387c59e7f4..1f21b0b574 100644 --- a/libraries/model-networking/src/model-networking/ModelCache.cpp +++ b/libraries/model-networking/src/model-networking/ModelCache.cpp @@ -97,6 +97,8 @@ void GeometryMappingResource::onGeometryMappingLoaded(bool success) { // Avoid holding onto extra references _geometryResource.reset(); + // Make sure connection will not trigger again + disconnect(_connection); // FIXME Should not have to do this } finishedLoading(success); From 7a153396318d226e293c70d3c26465c5a66b3baf Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Wed, 13 Apr 2016 15:38:03 -0700 Subject: [PATCH 26/40] Use AnimationCache for models --- interface/src/Application.cpp | 2 -- .../src/RenderableModelEntityItem.cpp | 8 ++--- .../src/RenderableModelEntityItem.h | 2 +- libraries/entities/src/ModelEntityItem.cpp | 31 ++++--------------- libraries/entities/src/ModelEntityItem.h | 9 ++---- 5 files changed, 13 insertions(+), 39 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index ebfdaff0e3..118f5425bd 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1238,8 +1238,6 @@ Application::~Application() { _physicsEngine->setCharacterController(nullptr); - ModelEntityItem::cleanupLoadedAnimations(); - // remove avatars from physics engine DependencyManager::get()->clearOtherAvatars(); VectorOfMotionStates motionStates; diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index 879ff01056..bbd1266513 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -274,11 +274,11 @@ bool RenderableModelEntityItem::getAnimationFrame() { if (!hasRenderAnimation() || !_jointMappingCompleted) { return false; } - AnimationPointer myAnimation = getAnimation(getRenderAnimationURL()); // FIXME: this could be optimized - if (myAnimation && myAnimation->isLoaded()) { - const QVector& frames = myAnimation->getFramesReference(); // NOTE: getFrames() is too heavy - auto& fbxJoints = myAnimation->getGeometry().joints; + if (_animation && _animation->isLoaded()) { + + const QVector& frames = _animation->getFramesReference(); // NOTE: getFrames() is too heavy + auto& fbxJoints = _animation->getGeometry().joints; int frameCount = frames.size(); if (frameCount > 0) { diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.h b/libraries/entities-renderer/src/RenderableModelEntityItem.h index bf55c829e9..59208d209d 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.h +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.h @@ -82,7 +82,7 @@ public: virtual int getJointIndex(const QString& name) const override; virtual QStringList getJointNames() const override; - // These operate on a copy of the renderAnimationProperties, so they can be accessed + // These operate on a copy of the animationProperties, so they can be accessed // without having the entityTree lock. bool hasRenderAnimation() const { return !_renderAnimationProperties.getURL().isEmpty(); } const QString& getRenderAnimationURL() const { return _renderAnimationProperties.getURL(); } diff --git a/libraries/entities/src/ModelEntityItem.cpp b/libraries/entities/src/ModelEntityItem.cpp index e5511c0b25..8e45f4dd5e 100644 --- a/libraries/entities/src/ModelEntityItem.cpp +++ b/libraries/entities/src/ModelEntityItem.cpp @@ -205,37 +205,18 @@ void ModelEntityItem::appendSubclassData(OctreePacketData* packetData, EncodeBit } -QMap ModelEntityItem::_loadedAnimations; // TODO: improve cleanup by leveraging the AnimationPointer(s) - -void ModelEntityItem::cleanupLoadedAnimations() { - foreach(AnimationPointer animation, _loadedAnimations) { - animation.clear(); - } - _loadedAnimations.clear(); -} - -AnimationPointer ModelEntityItem::getAnimation(const QString& url) { - AnimationPointer animation; - - // if we don't already have this model then create it and initialize it - if (_loadedAnimations.find(url) == _loadedAnimations.end()) { - animation = DependencyManager::get()->getAnimation(url); - _loadedAnimations[url] = animation; - } else { - animation = _loadedAnimations[url]; - } - return animation; -} - void ModelEntityItem::mapJoints(const QStringList& modelJointNames) { // if we don't have animation, or we're already joint mapped then bail early if (!hasAnimation() || jointsMapped()) { return; } - AnimationPointer myAnimation = getAnimation(_animationProperties.getURL()); - if (myAnimation && myAnimation->isLoaded()) { - QStringList animationJointNames = myAnimation->getJointNames(); + if (!_animation || _animation->getURL().toString() != getAnimationURL()) { + _animation = DependencyManager::get()->getAnimation(getAnimationURL()); + } + + if (_animation && _animation->isLoaded()) { + QStringList animationJointNames = _animation->getJointNames(); if (modelJointNames.size() > 0 && animationJointNames.size() > 0) { _jointMapping.resize(modelJointNames.size()); diff --git a/libraries/entities/src/ModelEntityItem.h b/libraries/entities/src/ModelEntityItem.h index d0e0909b27..82bb6ca47c 100644 --- a/libraries/entities/src/ModelEntityItem.h +++ b/libraries/entities/src/ModelEntityItem.h @@ -105,6 +105,7 @@ public: void mapJoints(const QStringList& modelJointNames); bool jointsMapped() const { return _jointMappingURL == getAnimationURL() && _jointMappingCompleted; } + AnimationPointer getAnimation() const { return _animation; } bool getAnimationIsPlaying() const { return _animationLoop.getRunning(); } float getAnimationCurrentFrame() const { return _animationLoop.getCurrentFrame(); } float getAnimationFPS() const { return _animationLoop.getFPS(); } @@ -115,8 +116,6 @@ public: virtual bool shouldBePhysical() const; - static void cleanupLoadedAnimations(); - virtual glm::vec3 getJointPosition(int jointIndex) const { return glm::vec3(); } virtual glm::quat getJointRotation(int jointIndex) const { return glm::quat(); } @@ -156,6 +155,7 @@ protected: QUrl _parsedModelURL; QString _compoundShapeURL; + AnimationPointer _animation; AnimationPropertyGroup _animationProperties; AnimationLoop _animationLoop; @@ -168,11 +168,6 @@ protected: bool _jointMappingCompleted; QVector _jointMapping; // domain is index into model-joints, range is index into animation-joints QString _jointMappingURL; - - static AnimationPointer getAnimation(const QString& url); - static QMap _loadedAnimations; - static AnimationCache _animationCache; - }; #endif // hifi_ModelEntityItem_h From b48503a82d9af4a6842ec88be9fc538b86c87b71 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 13 Apr 2016 19:52:50 -0700 Subject: [PATCH 27/40] only export joint values if they've been explicitly set by scripts --- .../src/RenderableModelEntityItem.cpp | 2 ++ .../entities/src/EntityItemProperties.cpp | 7 ------- libraries/entities/src/ModelEntityItem.cpp | 21 +++++++++++++++---- libraries/entities/src/ModelEntityItem.h | 4 ++++ 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index 879ff01056..7edd856688 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -748,6 +748,7 @@ glm::vec3 RenderableModelEntityItem::getAbsoluteJointTranslationInObjectFrame(in bool RenderableModelEntityItem::setAbsoluteJointRotationInObjectFrame(int index, const glm::quat& rotation) { bool result = false; _jointDataLock.withWriteLock([&] { + _jointRotationsExplicitlySet = true; resizeJointArrays(); if (index >= 0 && index < _absoluteJointRotationsInObjectFrame.size() && _absoluteJointRotationsInObjectFrame[index] != rotation) { @@ -764,6 +765,7 @@ bool RenderableModelEntityItem::setAbsoluteJointRotationInObjectFrame(int index, bool RenderableModelEntityItem::setAbsoluteJointTranslationInObjectFrame(int index, const glm::vec3& translation) { bool result = false; _jointDataLock.withWriteLock([&] { + _jointTranslationsExplicitlySet = true; resizeJointArrays(); if (index >= 0 && index < _absoluteJointTranslationsInObjectFrame.size() && _absoluteJointTranslationsInObjectFrame[index] != translation) { diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 32aac1efe5..92849d6e2f 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -541,13 +541,6 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_LOCAL_POSITION, localPosition); COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_LOCAL_ROTATION, localRotation); - COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_JOINT_ROTATIONS_SET, jointRotationsSet); - COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_JOINT_ROTATIONS, jointRotations); - COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_JOINT_TRANSLATIONS_SET, jointTranslationsSet); - COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_JOINT_TRANSLATIONS, jointTranslations); - - COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_QUERY_AA_CUBE, queryAACube); - // FIXME - I don't think these properties are supported any more //COPY_PROPERTY_TO_QSCRIPTVALUE(glowLevel); //COPY_PROPERTY_TO_QSCRIPTVALUE(localRenderAlpha); diff --git a/libraries/entities/src/ModelEntityItem.cpp b/libraries/entities/src/ModelEntityItem.cpp index e5511c0b25..6c7287ecb3 100644 --- a/libraries/entities/src/ModelEntityItem.cpp +++ b/libraries/entities/src/ModelEntityItem.cpp @@ -404,6 +404,7 @@ void ModelEntityItem::resizeJointArrays(int newSize) { void ModelEntityItem::setJointRotations(const QVector& rotations) { _jointDataLock.withWriteLock([&] { + _jointRotationsExplicitlySet = rotations.size() > 0; resizeJointArrays(rotations.size()); for (int index = 0; index < rotations.size(); index++) { if (_absoluteJointRotationsInObjectFrameSet[index]) { @@ -416,6 +417,7 @@ void ModelEntityItem::setJointRotations(const QVector& rotations) { void ModelEntityItem::setJointRotationsSet(const QVector& rotationsSet) { _jointDataLock.withWriteLock([&] { + _jointRotationsExplicitlySet = rotationsSet.size() > 0; resizeJointArrays(rotationsSet.size()); for (int index = 0; index < rotationsSet.size(); index++) { _absoluteJointRotationsInObjectFrameSet[index] = rotationsSet[index]; @@ -425,6 +427,7 @@ void ModelEntityItem::setJointRotationsSet(const QVector& rotationsSet) { void ModelEntityItem::setJointTranslations(const QVector& translations) { _jointDataLock.withWriteLock([&] { + _jointTranslationsExplicitlySet = translations.size() > 0; resizeJointArrays(translations.size()); for (int index = 0; index < translations.size(); index++) { if (_absoluteJointTranslationsInObjectFrameSet[index]) { @@ -437,6 +440,7 @@ void ModelEntityItem::setJointTranslations(const QVector& translation void ModelEntityItem::setJointTranslationsSet(const QVector& translationsSet) { _jointDataLock.withWriteLock([&] { + _jointTranslationsExplicitlySet = translationsSet.size() > 0; resizeJointArrays(translationsSet.size()); for (int index = 0; index < translationsSet.size(); index++) { _absoluteJointTranslationsInObjectFrameSet[index] = translationsSet[index]; @@ -447,7 +451,9 @@ void ModelEntityItem::setJointTranslationsSet(const QVector& translationsS QVector ModelEntityItem::getJointRotations() const { QVector result; _jointDataLock.withReadLock([&] { - result = _absoluteJointRotationsInObjectFrame; + if (_jointRotationsExplicitlySet) { + result = _absoluteJointRotationsInObjectFrame; + } }); return result; } @@ -455,15 +461,20 @@ QVector ModelEntityItem::getJointRotations() const { QVector ModelEntityItem::getJointRotationsSet() const { QVector result; _jointDataLock.withReadLock([&] { - result = _absoluteJointRotationsInObjectFrameSet; + if (_jointRotationsExplicitlySet) { + result = _absoluteJointRotationsInObjectFrameSet; + } }); + return result; } QVector ModelEntityItem::getJointTranslations() const { QVector result; _jointDataLock.withReadLock([&] { - result = _absoluteJointTranslationsInObjectFrame; + if (_jointTranslationsExplicitlySet) { + result = _absoluteJointTranslationsInObjectFrame; + } }); return result; } @@ -471,7 +482,9 @@ QVector ModelEntityItem::getJointTranslations() const { QVector ModelEntityItem::getJointTranslationsSet() const { QVector result; _jointDataLock.withReadLock([&] { - result = _absoluteJointTranslationsInObjectFrameSet; + if (_jointTranslationsExplicitlySet) { + result = _absoluteJointTranslationsInObjectFrameSet; + } }); return result; } diff --git a/libraries/entities/src/ModelEntityItem.h b/libraries/entities/src/ModelEntityItem.h index d0e0909b27..640c00f14d 100644 --- a/libraries/entities/src/ModelEntityItem.h +++ b/libraries/entities/src/ModelEntityItem.h @@ -140,9 +140,13 @@ protected: // they aren't currently updated from data in the model/rig, and they don't have a direct effect // on what's rendered. ReadWriteLockable _jointDataLock; + + bool _jointRotationsExplicitlySet { false }; // were the joints set as a property or just side effect of animations QVector _absoluteJointRotationsInObjectFrame; QVector _absoluteJointRotationsInObjectFrameSet; // ever set? QVector _absoluteJointRotationsInObjectFrameDirty; // needs a relay to model/rig? + + bool _jointTranslationsExplicitlySet { false }; // were the joints set as a property or just side effect of animations QVector _absoluteJointTranslationsInObjectFrame; QVector _absoluteJointTranslationsInObjectFrameSet; // ever set? QVector _absoluteJointTranslationsInObjectFrameDirty; // needs a relay to model/rig? From 55002b8d5ec30d0d509c58a8c25d76005eabb77c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 14 Apr 2016 09:27:43 -0700 Subject: [PATCH 28/40] add ice server address to hearbeat if full networking is on --- domain-server/src/DomainServer.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 3b2f13c668..41089ff44f 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -1046,6 +1046,7 @@ void DomainServer::sendHeartbeatToDataServer(const QString& networkAddress) { // setup the domain object to send to the data server const QString PUBLIC_NETWORK_ADDRESS_KEY = "network_address"; const QString AUTOMATIC_NETWORKING_KEY = "automatic_networking"; + const QString ICE_SERVER_ADDRESS = "ice_server_address"; QJsonObject domainObject; if (!networkAddress.isEmpty()) { @@ -1054,6 +1055,11 @@ void DomainServer::sendHeartbeatToDataServer(const QString& networkAddress) { domainObject[AUTOMATIC_NETWORKING_KEY] = _automaticNetworkingSetting; + // if we're using full automatic networking and we have a current ice-server socket, use that now + if (_automaticNetworkingSetting == FULL_AUTOMATIC_NETWORKING_VALUE && !_iceServerSocket.isNull()) { + domainObject[ICE_SERVER_ADDRESS] = _iceServerSocket.getAddress().toString(); + } + // add a flag to indicate if this domain uses restricted access - for now that will exclude it from listings const QString RESTRICTED_ACCESS_FLAG = "restricted"; @@ -1079,7 +1085,7 @@ void DomainServer::sendHeartbeatToDataServer(const QString& networkAddress) { QString domainUpdateJSON = QString("{\"domain\": %1 }").arg(QString(QJsonDocument(domainObject).toJson())); AccountManager::getInstance().sendRequest(DOMAIN_UPDATE.arg(uuidStringWithoutCurlyBraces(domainID)), - AccountManagerAuth::Required, + AccountManagerAuth::Optional, QNetworkAccessManager::PutOperation, JSONCallbackParameters(), domainUpdateJSON.toUtf8()); From 3f0ffc0aff1823455eaaeb872feeb219945f140f Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 14 Apr 2016 10:33:17 -0700 Subject: [PATCH 29/40] update ice-server address with API, add failed and clear denials --- domain-server/src/DomainServer.cpp | 70 +++++++++++++++++++++++++----- domain-server/src/DomainServer.h | 6 +++ 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 41089ff44f..5df9492f87 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -241,7 +241,7 @@ void DomainServer::optionallyGetTemporaryName(const QStringList& arguments) { // so fire off that request now auto& accountManager = AccountManager::getInstance(); - // ask our auth endpoint for our balance + // get callbacks for temporary domain result JSONCallbackParameters callbackParameters; callbackParameters.jsonCallbackReceiver = this; callbackParameters.jsonCallbackMethod = "handleTempDomainSuccess"; @@ -434,7 +434,9 @@ void DomainServer::setupAutomaticNetworking() { setupICEHeartbeatForFullNetworking(); } - if (!resetAccountManagerAccessToken()) { + _hasAccessToken = resetAccountManagerAccessToken(); + + if (!_hasAccessToken) { qDebug() << "Will not send heartbeat to Metaverse API without an access token."; qDebug() << "If this is not a temporary domain add an access token to your config file or via the web interface."; @@ -1038,15 +1040,16 @@ void DomainServer::performIPAddressUpdate(const HifiSockAddr& newPublicSockAddr) sendHeartbeatToDataServer(newPublicSockAddr.getAddress().toString()); } + void DomainServer::sendHeartbeatToDataServer(const QString& networkAddress) { const QString DOMAIN_UPDATE = "/api/v1/domains/%1"; + auto nodeList = DependencyManager::get(); const QUuid& domainID = nodeList->getSessionUUID(); // setup the domain object to send to the data server const QString PUBLIC_NETWORK_ADDRESS_KEY = "network_address"; const QString AUTOMATIC_NETWORKING_KEY = "automatic_networking"; - const QString ICE_SERVER_ADDRESS = "ice_server_address"; QJsonObject domainObject; if (!networkAddress.isEmpty()) { @@ -1055,11 +1058,6 @@ void DomainServer::sendHeartbeatToDataServer(const QString& networkAddress) { domainObject[AUTOMATIC_NETWORKING_KEY] = _automaticNetworkingSetting; - // if we're using full automatic networking and we have a current ice-server socket, use that now - if (_automaticNetworkingSetting == FULL_AUTOMATIC_NETWORKING_VALUE && !_iceServerSocket.isNull()) { - domainObject[ICE_SERVER_ADDRESS] = _iceServerSocket.getAddress().toString(); - } - // add a flag to indicate if this domain uses restricted access - for now that will exclude it from listings const QString RESTRICTED_ACCESS_FLAG = "restricted"; @@ -1085,12 +1083,52 @@ void DomainServer::sendHeartbeatToDataServer(const QString& networkAddress) { QString domainUpdateJSON = QString("{\"domain\": %1 }").arg(QString(QJsonDocument(domainObject).toJson())); AccountManager::getInstance().sendRequest(DOMAIN_UPDATE.arg(uuidStringWithoutCurlyBraces(domainID)), - AccountManagerAuth::Optional, + AccountManagerAuth::Required, QNetworkAccessManager::PutOperation, JSONCallbackParameters(), domainUpdateJSON.toUtf8()); } +void DomainServer::sendICEServerAddressToMetaverseAPI() { + if (!_iceServerSocket.isNull()) { + auto nodeList = DependencyManager::get(); + const QUuid& domainID = nodeList->getSessionUUID(); + + const QString ICE_SERVER_ADDRESS = "ice_server_address"; + + QJsonObject domainObject; + + // we're using full automatic networking and we have a current ice-server socket, use that now + domainObject[ICE_SERVER_ADDRESS] = _iceServerSocket.getAddress().toString(); + + QString domainUpdateJSON = QString("{\"domain\": %1 }").arg(QString(QJsonDocument(domainObject).toJson())); + + // make sure we hear about failure so we can retry + JSONCallbackParameters callbackParameters; + callbackParameters.errorCallbackReceiver = this; + callbackParameters.errorCallbackMethod = "handleFailedICEServerAddressUpdate"; + + qDebug() << "Updating ice-server address in High Fidelity Metaverse API to" << _iceServerSocket.getAddress().toString(); + + static const QString DOMAIN_ICE_ADDRESS_UPDATE = "/api/v1/domains/%1/ice_server_address"; + + AccountManager::getInstance().sendRequest(DOMAIN_ICE_ADDRESS_UPDATE.arg(uuidStringWithoutCurlyBraces(domainID)), + AccountManagerAuth::Optional, + QNetworkAccessManager::PutOperation, + callbackParameters, + domainUpdateJSON.toUtf8()); + } +} + +void DomainServer::handleFailedICEServerAddressUpdate(QNetworkReply& requestReply) { + const int ICE_SERVER_UPDATE_RETRY_MS = 2 * 1000; + + qWarning() << "Failed to update ice-server address with High Fidelity Metaverse - error was" << requestReply.errorString(); + qWarning() << "\tRe-attempting in" << ICE_SERVER_UPDATE_RETRY_MS / 1000 << "seconds"; + + QTimer::singleShot(ICE_SERVER_UPDATE_RETRY_MS, this, SLOT(sendICEServerAddressToMetaverseAPI())); +} + void DomainServer::sendHeartbeatToIceServer() { if (!_iceServerSocket.getAddress().isNull()) { @@ -1119,6 +1157,9 @@ void DomainServer::sendHeartbeatToIceServer() { qWarning() << "There have been" << _noReplyICEHeartbeats - 1 << "heartbeats sent with no reply from the ice-server"; qWarning() << "Clearing the current ice-server socket and selecting a new candidate ice-server"; + // add the current address to our list of failed addresses + _failedIceServerAddresses << _iceServerSocket.getAddress(); + // if we've failed to hear back for three heartbeats, we clear the current ice-server socket and attempt // to randomize a new one _iceServerSocket.clear(); @@ -2062,8 +2103,7 @@ void DomainServer::processNodeDisconnectRequestPacket(QSharedPointer message) { static const int NUM_HEARTBEAT_DENIALS_FOR_KEYPAIR_REGEN = 3; - static int numHeartbeatDenials = 0; - if (++numHeartbeatDenials > NUM_HEARTBEAT_DENIALS_FOR_KEYPAIR_REGEN) { + if (++_numHeartbeatDenials > NUM_HEARTBEAT_DENIALS_FOR_KEYPAIR_REGEN) { qDebug() << "Received" << NUM_HEARTBEAT_DENIALS_FOR_KEYPAIR_REGEN << "heartbeat denials from ice-server" << "- re-generating keypair now"; @@ -2072,7 +2112,7 @@ void DomainServer::processICEServerHeartbeatDenialPacket(QSharedPointergetSessionUUID()); // reset our number of heartbeat denials - numHeartbeatDenials = 0; + _numHeartbeatDenials = 0; } // even though we can't get into this ice-server it is responding to us, so we reset our number of no-reply heartbeats @@ -2172,6 +2212,12 @@ void DomainServer::randomizeICEServerAddress() { _iceServerSocket = HifiSockAddr { candidateICEAddresses[indexToTry], ICE_SERVER_DEFAULT_PORT }; qInfo() << "Set candidate ice-server socket to" << _iceServerSocket; + // clear our number of hearbeat denials, this should be re-set on ice-server change + _numHeartbeatDenials = 0; + // immediately fire an ICE heartbeat once we've picked a candidate ice-server sendHeartbeatToIceServer(); + + // immediately send an update to the metaverse API when our ice-server changes + sendICEServerAddressToMetaverseAPI(); } diff --git a/domain-server/src/DomainServer.h b/domain-server/src/DomainServer.h index 1165c76559..1ece2e30dd 100644 --- a/domain-server/src/DomainServer.h +++ b/domain-server/src/DomainServer.h @@ -86,6 +86,9 @@ private slots: void updateICEServerAddresses(); void handleICEHostInfo(const QHostInfo& hostInfo); + void sendICEServerAddressToMetaverseAPI(); + void handleFailedICEServerAddressUpdate(QNetworkReply& requestReply); + signals: void iceServerChanged(); @@ -172,8 +175,11 @@ private: QTimer* _iceAddressLookupTimer { nullptr }; // this looks like a dangling pointer but is parented to the DomainServer int _iceAddressLookupID { -1 }; int _noReplyICEHeartbeats { 0 }; + int _numHeartbeatDenials { 0 }; bool _connectedToICEServer { false }; + bool _hasAccessToken { false }; + friend class DomainGatekeeper; }; From dcf28937cfd022af646c71e2538f77a14f7834f9 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 11 Apr 2016 21:19:55 -0700 Subject: [PATCH 30/40] cleanup cached Physics::sessionUUID --- interface/src/Application.cpp | 7 ++----- libraries/physics/src/EntityMotionState.cpp | 20 ++++++++++--------- libraries/physics/src/EntityMotionState.h | 6 +++--- .../physics/src/PhysicalEntitySimulation.cpp | 12 +++++------ .../physics/src/PhysicalEntitySimulation.h | 2 +- libraries/physics/src/PhysicsEngine.cpp | 11 +++++----- libraries/physics/src/PhysicsEngine.h | 4 ---- 7 files changed, 28 insertions(+), 34 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 65bbfd7d6c..eb9a0c8661 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -3446,7 +3446,7 @@ void Application::update(float deltaTime) { getEntities()->getTree()->withWriteLock([&] { PerformanceTimer perfTimer("handleOutgoingChanges"); const VectorOfMotionStates& outgoingChanges = _physicsEngine->getOutgoingChanges(); - _entitySimulation.handleOutgoingChanges(outgoingChanges, Physics::getSessionUUID()); + _entitySimulation.handleOutgoingChanges(outgoingChanges); avatarManager->handleOutgoingChanges(outgoingChanges); }); @@ -4542,10 +4542,7 @@ bool Application::acceptURL(const QString& urlString, bool defaultUpload) { } void Application::setSessionUUID(const QUuid& sessionUUID) const { - // HACK: until we swap the library dependency order between physics and entities - // we cache the sessionID in two distinct places for physics. - Physics::setSessionUUID(sessionUUID); // TODO: remove this one - _physicsEngine->setSessionUUID(sessionUUID); + Physics::setSessionUUID(sessionUUID); } bool Application::askToSetAvatarUrl(const QString& url) { diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index a9dcb4a16c..5dd6edddda 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -223,7 +223,6 @@ void EntityMotionState::setWorldTransform(const btTransform& worldTrans) { if (_entity->getSimulatorID().isNull()) { _loopsWithoutOwner++; - if (_loopsWithoutOwner > LOOPS_FOR_SIMULATION_ORPHAN && usecTimestampNow() > _nextOwnershipBid) { upgradeOutgoingPriority(VOLUNTEER_SIMULATION_PRIORITY); } @@ -255,11 +254,13 @@ btCollisionShape* EntityMotionState::computeNewShape() { return getShapeManager()->getShape(shapeInfo); } -bool EntityMotionState::isCandidateForOwnership(const QUuid& sessionID) const { +bool EntityMotionState::isCandidateForOwnership() const { assert(_body); assert(_entity); assert(entityTreeIsLocked()); - return _outgoingPriority != 0 || sessionID == _entity->getSimulatorID() || _entity->actionDataNeedsTransmit(); + return _outgoingPriority != 0 + || Physics::getSessionUUID() == _entity->getSimulatorID() + || _entity->actionDataNeedsTransmit(); } bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) { @@ -384,7 +385,7 @@ bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) { return (fabsf(glm::dot(actualRotation, _serverRotation)) < MIN_ROTATION_DOT); } -bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep, const QUuid& sessionID) { +bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep) { // NOTE: we expect _entity and _body to be valid in this context, since shouldSendUpdate() is only called // after doesNotNeedToSendUpdate() returns false and that call should return 'true' if _entity or _body are NULL. assert(_entity); @@ -399,7 +400,7 @@ bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep, const QUuid& s return true; } - if (_entity->getSimulatorID() != sessionID) { + if (_entity->getSimulatorID() != Physics::getSessionUUID()) { // we don't own the simulation bool shouldBid = _outgoingPriority > 0 && // but we would like to own it and usecTimestampNow() > _nextOwnershipBid; // it is time to bid again @@ -415,7 +416,7 @@ bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep, const QUuid& s return remoteSimulationOutOfSync(simulationStep); } -void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, const QUuid& sessionID, uint32_t step) { +void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_t step) { assert(_entity); assert(entityTreeIsLocked()); @@ -514,9 +515,10 @@ void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, const Q // but we remember we do still own it... and rely on the server to tell us we don't properties.clearSimulationOwner(); _outgoingPriority = 0; - } else if (sessionID != _entity->getSimulatorID()) { + } else if (Physics::getSessionUUID() != _entity->getSimulatorID()) { // we don't own the simulation for this entity yet, but we're sending a bid for it - properties.setSimulationOwner(sessionID, glm::max(_outgoingPriority, VOLUNTEER_SIMULATION_PRIORITY)); + properties.setSimulationOwner(Physics::getSessionUUID(), + glm::max(_outgoingPriority, VOLUNTEER_SIMULATION_PRIORITY)); _nextOwnershipBid = now + USECS_BETWEEN_OWNERSHIP_BIDS; _outgoingPriority = 0; // reset outgoing priority whenever we bid } else if (_outgoingPriority != _entity->getSimulationPriority()) { @@ -526,7 +528,7 @@ void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, const Q properties.clearSimulationOwner(); } else { // we just need to change the priority - properties.setSimulationOwner(sessionID, _outgoingPriority); + properties.setSimulationOwner(Physics::getSessionUUID(), _outgoingPriority); } } diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index ac16ec6d5d..8150de22d6 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -43,10 +43,10 @@ public: // this relays outgoing position/rotation to the EntityItem virtual void setWorldTransform(const btTransform& worldTrans) override; - bool isCandidateForOwnership(const QUuid& sessionID) const; + bool isCandidateForOwnership() const; bool remoteSimulationOutOfSync(uint32_t simulationStep); - bool shouldSendUpdate(uint32_t simulationStep, const QUuid& sessionID); - void sendUpdate(OctreeEditPacketSender* packetSender, const QUuid& sessionID, uint32_t step); + bool shouldSendUpdate(uint32_t simulationStep); + void sendUpdate(OctreeEditPacketSender* packetSender, uint32_t step); virtual uint32_t getIncomingDirtyFlags() override; virtual void clearIncomingDirtyFlags() override; diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 2d219915c8..70ca512646 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -251,7 +251,7 @@ void PhysicalEntitySimulation::getObjectsToChange(VectorOfMotionStates& result) _pendingChanges.clear(); } -void PhysicalEntitySimulation::handleOutgoingChanges(const VectorOfMotionStates& motionStates, const QUuid& sessionID) { +void PhysicalEntitySimulation::handleOutgoingChanges(const VectorOfMotionStates& motionStates) { QMutexLocker lock(&_mutex); // walk the motionStates looking for those that correspond to entities @@ -261,7 +261,7 @@ void PhysicalEntitySimulation::handleOutgoingChanges(const VectorOfMotionStates& EntityMotionState* entityState = static_cast(state); EntityItemPointer entity = entityState->getEntity(); assert(entity.get()); - if (entityState->isCandidateForOwnership(sessionID)) { + if (entityState->isCandidateForOwnership()) { _outgoingChanges.insert(entityState); } _entitiesToSort.insert(entity); @@ -272,7 +272,7 @@ void PhysicalEntitySimulation::handleOutgoingChanges(const VectorOfMotionStates& if (_lastStepSendPackets != numSubsteps) { _lastStepSendPackets = numSubsteps; - if (sessionID.isNull()) { + if (Physics::getSessionUUID().isNull()) { // usually don't get here, but if so --> nothing to do _outgoingChanges.clear(); return; @@ -282,12 +282,12 @@ void PhysicalEntitySimulation::handleOutgoingChanges(const VectorOfMotionStates& QSet::iterator stateItr = _outgoingChanges.begin(); while (stateItr != _outgoingChanges.end()) { EntityMotionState* state = *stateItr; - if (!state->isCandidateForOwnership(sessionID)) { + if (!state->isCandidateForOwnership()) { // prune stateItr = _outgoingChanges.erase(stateItr); - } else if (state->shouldSendUpdate(numSubsteps, sessionID)) { + } else if (state->shouldSendUpdate(numSubsteps)) { // update - state->sendUpdate(_entityPacketSender, sessionID, numSubsteps); + state->sendUpdate(_entityPacketSender, numSubsteps); ++stateItr; } else { ++stateItr; diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index 7138fb924c..935ee57115 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -54,7 +54,7 @@ public: void setObjectsToChange(const VectorOfMotionStates& objectsToChange); void getObjectsToChange(VectorOfMotionStates& result); - void handleOutgoingChanges(const VectorOfMotionStates& motionStates, const QUuid& sessionID); + void handleOutgoingChanges(const VectorOfMotionStates& motionStates); void handleCollisionEvents(const CollisionEvents& collisionEvents); EntityEditPacketSender* getPacketSender() { return _entityPacketSender; } diff --git a/libraries/physics/src/PhysicsEngine.cpp b/libraries/physics/src/PhysicsEngine.cpp index 0040c19c3d..9312504c72 100644 --- a/libraries/physics/src/PhysicsEngine.cpp +++ b/libraries/physics/src/PhysicsEngine.cpp @@ -20,7 +20,6 @@ PhysicsEngine::PhysicsEngine(const glm::vec3& offset) : _originOffset(offset), - _sessionID(), _myAvatarController(nullptr) { } @@ -286,20 +285,20 @@ void PhysicsEngine::doOwnershipInfection(const btCollisionObject* objectA, const ObjectMotionState* motionStateB = static_cast(objectB->getUserPointer()); if (motionStateB && - ((motionStateA && motionStateA->getSimulatorID() == _sessionID && !objectA->isStaticObject()) || + ((motionStateA && motionStateA->getSimulatorID() == Physics::getSessionUUID() && !objectA->isStaticObject()) || (objectA == characterObject))) { // NOTE: we might own the simulation of a kinematic object (A) // but we don't claim ownership of kinematic objects (B) based on collisions here. - if (!objectB->isStaticOrKinematicObject() && motionStateB->getSimulatorID() != _sessionID) { + if (!objectB->isStaticOrKinematicObject() && motionStateB->getSimulatorID() != Physics::getSessionUUID()) { quint8 priorityA = motionStateA ? motionStateA->getSimulationPriority() : PERSONAL_SIMULATION_PRIORITY; motionStateB->bump(priorityA); } } else if (motionStateA && - ((motionStateB && motionStateB->getSimulatorID() == _sessionID && !objectB->isStaticObject()) || + ((motionStateB && motionStateB->getSimulatorID() == Physics::getSessionUUID() && !objectB->isStaticObject()) || (objectB == characterObject))) { // SIMILARLY: we might own the simulation of a kinematic object (B) // but we don't claim ownership of kinematic objects (A) based on collisions here. - if (!objectA->isStaticOrKinematicObject() && motionStateA->getSimulatorID() != _sessionID) { + if (!objectA->isStaticOrKinematicObject() && motionStateA->getSimulatorID() != Physics::getSessionUUID()) { quint8 priorityB = motionStateB ? motionStateB->getSimulationPriority() : PERSONAL_SIMULATION_PRIORITY; motionStateA->bump(priorityB); } @@ -333,7 +332,7 @@ void PhysicsEngine::updateContactMap() { _contactMap[ContactKey(a, b)].update(_numContactFrames, contactManifold->getContactPoint(0)); } - if (!_sessionID.isNull()) { + if (!Physics::getSessionUUID().isNull()) { doOwnershipInfection(objectA, objectB); } } diff --git a/libraries/physics/src/PhysicsEngine.h b/libraries/physics/src/PhysicsEngine.h index f644d6f5b2..38caf079e7 100644 --- a/libraries/physics/src/PhysicsEngine.h +++ b/libraries/physics/src/PhysicsEngine.h @@ -87,8 +87,6 @@ public: void removeAction(const QUuid actionID); void forEachAction(std::function actor); - void setSessionUUID(const QUuid& sessionID) { _sessionID = sessionID; } - private: void addObjectToDynamicsWorld(ObjectMotionState* motionState); @@ -112,7 +110,6 @@ private: QHash _objectActions; glm::vec3 _originOffset; - QUuid _sessionID; CharacterController* _myAvatarController; @@ -121,7 +118,6 @@ private: bool _dumpNextStats = false; bool _hasOutgoingChanges = false; - }; typedef std::shared_ptr PhysicsEnginePointer; From bb59860cfe045838f344fdcbe5db1be5f2378803 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 11 Apr 2016 23:15:22 -0700 Subject: [PATCH 31/40] update Aabb's of static objects when they move --- libraries/physics/src/EntityMotionState.cpp | 4 +-- libraries/physics/src/EntityMotionState.h | 2 +- libraries/physics/src/ObjectMotionState.cpp | 12 ++++++-- libraries/physics/src/ObjectMotionState.h | 9 +++--- libraries/physics/src/PhysicsEngine.cpp | 29 +++++++++++++++---- libraries/physics/src/PhysicsEngine.h | 5 ++-- libraries/shared/src/PhysicsCollisionGroups.h | 4 +-- 7 files changed, 45 insertions(+), 20 deletions(-) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 5dd6edddda..cd6c54675d 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -94,7 +94,7 @@ void EntityMotionState::updateServerPhysicsVariables() { } // virtual -bool EntityMotionState::handleEasyChanges(uint32_t& flags) { +void EntityMotionState::handleEasyChanges(uint32_t& flags) { assert(entityTreeIsLocked()); updateServerPhysicsVariables(); ObjectMotionState::handleEasyChanges(flags); @@ -137,8 +137,6 @@ bool EntityMotionState::handleEasyChanges(uint32_t& flags) { if ((flags & Simulation::DIRTY_PHYSICS_ACTIVATION) && !_body->isActive()) { _body->activate(); } - - return true; } diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index 8150de22d6..938f3e84c1 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -29,7 +29,7 @@ public: virtual ~EntityMotionState(); void updateServerPhysicsVariables(); - virtual bool handleEasyChanges(uint32_t& flags) override; + virtual void handleEasyChanges(uint32_t& flags) override; virtual bool handleHardAndEasyChanges(uint32_t& flags, PhysicsEngine* engine) override; /// \return PhysicsMotionType based on params set in EntityItem diff --git a/libraries/physics/src/ObjectMotionState.cpp b/libraries/physics/src/ObjectMotionState.cpp index 482c3146f8..0468e02cb8 100644 --- a/libraries/physics/src/ObjectMotionState.cpp +++ b/libraries/physics/src/ObjectMotionState.cpp @@ -164,7 +164,7 @@ void ObjectMotionState::setRigidBody(btRigidBody* body) { } } -bool ObjectMotionState::handleEasyChanges(uint32_t& flags) { +void ObjectMotionState::handleEasyChanges(uint32_t& flags) { if (flags & Simulation::DIRTY_POSITION) { btTransform worldTrans = _body->getWorldTransform(); btVector3 newPosition = glmToBullet(getObjectPosition()); @@ -183,6 +183,10 @@ bool ObjectMotionState::handleEasyChanges(uint32_t& flags) { worldTrans.setRotation(newRotation); } _body->setWorldTransform(worldTrans); + if (!(flags & HARD_DIRTY_PHYSICS_FLAGS) && _body->isStaticObject()) { + // force activate static body so its Aabb is updated later + _body->activate(true); + } } else if (flags & Simulation::DIRTY_ROTATION) { btTransform worldTrans = _body->getWorldTransform(); btQuaternion newRotation = glmToBullet(getObjectRotation()); @@ -192,6 +196,10 @@ bool ObjectMotionState::handleEasyChanges(uint32_t& flags) { } worldTrans.setRotation(newRotation); _body->setWorldTransform(worldTrans); + if (!(flags & HARD_DIRTY_PHYSICS_FLAGS) && _body->isStaticObject()) { + // force activate static body so its Aabb is updated later + _body->activate(true); + } } if (flags & Simulation::DIRTY_LINEAR_VELOCITY) { @@ -232,8 +240,6 @@ bool ObjectMotionState::handleEasyChanges(uint32_t& flags) { if (flags & Simulation::DIRTY_MASS) { updateBodyMassProperties(); } - - return true; } bool ObjectMotionState::handleHardAndEasyChanges(uint32_t& flags, PhysicsEngine* engine) { diff --git a/libraries/physics/src/ObjectMotionState.h b/libraries/physics/src/ObjectMotionState.h index bb78eb12d6..4293df6b3f 100644 --- a/libraries/physics/src/ObjectMotionState.h +++ b/libraries/physics/src/ObjectMotionState.h @@ -50,11 +50,12 @@ const uint32_t HARD_DIRTY_PHYSICS_FLAGS = (uint32_t)(Simulation::DIRTY_MOTION_TY Simulation::DIRTY_COLLISION_GROUP); const uint32_t EASY_DIRTY_PHYSICS_FLAGS = (uint32_t)(Simulation::DIRTY_TRANSFORM | Simulation::DIRTY_VELOCITIES | Simulation::DIRTY_MASS | Simulation::DIRTY_MATERIAL | - Simulation::DIRTY_SIMULATOR_ID | Simulation::DIRTY_SIMULATION_OWNERSHIP_PRIORITY); + Simulation::DIRTY_SIMULATOR_ID | Simulation::DIRTY_SIMULATION_OWNERSHIP_PRIORITY | + Simulation::DIRTY_PHYSICS_ACTIVATION); + // These are the set of incoming flags that the PhysicsEngine needs to hear about: -const uint32_t DIRTY_PHYSICS_FLAGS = (uint32_t)(HARD_DIRTY_PHYSICS_FLAGS | EASY_DIRTY_PHYSICS_FLAGS | - Simulation::DIRTY_PHYSICS_ACTIVATION); +const uint32_t DIRTY_PHYSICS_FLAGS = (uint32_t)(HARD_DIRTY_PHYSICS_FLAGS | EASY_DIRTY_PHYSICS_FLAGS); // These are the outgoing flags that the PhysicsEngine can affect: const uint32_t OUTGOING_DIRTY_PHYSICS_FLAGS = Simulation::DIRTY_TRANSFORM | Simulation::DIRTY_VELOCITIES; @@ -80,7 +81,7 @@ public: ObjectMotionState(btCollisionShape* shape); ~ObjectMotionState(); - virtual bool handleEasyChanges(uint32_t& flags); + virtual void handleEasyChanges(uint32_t& flags); virtual bool handleHardAndEasyChanges(uint32_t& flags, PhysicsEngine* engine); void updateBodyMaterialProperties(); diff --git a/libraries/physics/src/PhysicsEngine.cpp b/libraries/physics/src/PhysicsEngine.cpp index 9312504c72..54419a9c1a 100644 --- a/libraries/physics/src/PhysicsEngine.cpp +++ b/libraries/physics/src/PhysicsEngine.cpp @@ -49,6 +49,13 @@ void PhysicsEngine::init() { // default gravity of the world is zero, so each object must specify its own gravity // TODO: set up gravity zones _dynamicsWorld->setGravity(btVector3(0.0f, 0.0f, 0.0f)); + + // By default Bullet will update the Aabb's of all objects every frame, even statics. + // This can waste CPU cycles so we configure Bullet to only update ACTIVE objects here. + // However, this means when a static object is moved we must manually update its Aabb + // in order for its broadphase collision queries to work correctly. Look at how we use + // _activeStaticBodies to track and update the Aabb's of moved static objects. + _dynamicsWorld->setForceUpdateAllAabbs(false); } } @@ -188,12 +195,18 @@ VectorOfMotionStates PhysicsEngine::changeObjects(const VectorOfMotionStates& ob stillNeedChange.push_back(object); } } else if (flags & EASY_DIRTY_PHYSICS_FLAGS) { - if (object->handleEasyChanges(flags)) { - object->clearIncomingDirtyFlags(); - } else { - stillNeedChange.push_back(object); - } + object->handleEasyChanges(flags); + object->clearIncomingDirtyFlags(); } + if (object->getMotionType() == MOTION_TYPE_STATIC && object->isActive()) { + _activeStaticBodies.push_back(object->getRigidBody()); + } + } + // active static bodies have changed (in an Easy way) and need their Aabbs updated + // but we've configured Bullet to NOT update them automatically (for improved performance) + // so we must do it ourselves + for (size_t i = 0; i < _activeStaticBodies.size(); ++i) { + _dynamicsWorld->updateSingleAabb(_activeStaticBodies[i]); } return stillNeedChange; } @@ -387,6 +400,12 @@ const CollisionEvents& PhysicsEngine::getCollisionEvents() { const VectorOfMotionStates& PhysicsEngine::getOutgoingChanges() { BT_PROFILE("copyOutgoingChanges"); + // Bullet will not deactivate static objects (it doesn't expect them to be active) + // so we must deactivate them ourselves + for (size_t i = 0; i < _activeStaticBodies.size(); ++i) { + _activeStaticBodies[i]->forceActivationState(ISLAND_SLEEPING); + } + _activeStaticBodies.clear(); _dynamicsWorld->synchronizeMotionStates(); _hasOutgoingChanges = false; return _dynamicsWorld->getChangedMotionStates(); diff --git a/libraries/physics/src/PhysicsEngine.h b/libraries/physics/src/PhysicsEngine.h index 38caf079e7..72a41b391a 100644 --- a/libraries/physics/src/PhysicsEngine.h +++ b/libraries/physics/src/PhysicsEngine.h @@ -13,9 +13,9 @@ #define hifi_PhysicsEngine_h #include +#include #include -#include #include #include @@ -41,7 +41,7 @@ public: }; typedef std::map ContactMap; -typedef QVector CollisionEvents; +typedef std::vector CollisionEvents; class PhysicsEngine { public: @@ -108,6 +108,7 @@ private: ContactMap _contactMap; CollisionEvents _collisionEvents; QHash _objectActions; + std::vector _activeStaticBodies; glm::vec3 _originOffset; diff --git a/libraries/shared/src/PhysicsCollisionGroups.h b/libraries/shared/src/PhysicsCollisionGroups.h index 6d320e69cb..794f338dc5 100644 --- a/libraries/shared/src/PhysicsCollisionGroups.h +++ b/libraries/shared/src/PhysicsCollisionGroups.h @@ -51,10 +51,10 @@ const int16_t BULLET_COLLISION_GROUP_COLLISIONLESS = 1 << 14; const int16_t BULLET_COLLISION_MASK_DEFAULT = ~ BULLET_COLLISION_GROUP_COLLISIONLESS; // STATIC does not collide with itself (as optimization of physics simulation) -const int16_t BULLET_COLLISION_MASK_STATIC = ~ (BULLET_COLLISION_GROUP_COLLISIONLESS | BULLET_COLLISION_GROUP_STATIC); +const int16_t BULLET_COLLISION_MASK_STATIC = ~ (BULLET_COLLISION_GROUP_COLLISIONLESS | BULLET_COLLISION_GROUP_KINEMATIC | BULLET_COLLISION_GROUP_STATIC); const int16_t BULLET_COLLISION_MASK_DYNAMIC = BULLET_COLLISION_MASK_DEFAULT; -const int16_t BULLET_COLLISION_MASK_KINEMATIC = BULLET_COLLISION_MASK_DEFAULT; +const int16_t BULLET_COLLISION_MASK_KINEMATIC = BULLET_COLLISION_MASK_STATIC; // MY_AVATAR does not collide with itself const int16_t BULLET_COLLISION_MASK_MY_AVATAR = ~(BULLET_COLLISION_GROUP_COLLISIONLESS | BULLET_COLLISION_GROUP_MY_AVATAR); From 4d7efbc43fa682ac7d4a8cd2dd9a49074eb7558f Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 12 Apr 2016 08:50:42 -0700 Subject: [PATCH 32/40] faster isNaN() and friends --- libraries/shared/src/GLMHelpers.cpp | 8 -------- libraries/shared/src/GLMHelpers.h | 4 ++-- libraries/shared/src/SharedUtil.cpp | 17 +---------------- libraries/shared/src/SharedUtil.h | 6 +++--- libraries/shared/src/Transform.cpp | 4 ---- libraries/shared/src/Transform.h | 2 +- 6 files changed, 7 insertions(+), 34 deletions(-) diff --git a/libraries/shared/src/GLMHelpers.cpp b/libraries/shared/src/GLMHelpers.cpp index d21d88d212..53abb3827d 100644 --- a/libraries/shared/src/GLMHelpers.cpp +++ b/libraries/shared/src/GLMHelpers.cpp @@ -463,14 +463,6 @@ glm::vec2 getFacingDir2D(const glm::mat4& m) { } } -bool isNaN(glm::vec3 value) { - return isNaN(value.x) || isNaN(value.y) || isNaN(value.z); -} - -bool isNaN(glm::quat value) { - return isNaN(value.w) || isNaN(value.x) || isNaN(value.y) || isNaN(value.z); -} - glm::mat4 orthoInverse(const glm::mat4& m) { glm::mat4 r = m; r[3] = glm::vec4(0.0f, 0.0f, 0.0f, 1.0f); diff --git a/libraries/shared/src/GLMHelpers.h b/libraries/shared/src/GLMHelpers.h index 469ca1fc81..8b1446d4e5 100644 --- a/libraries/shared/src/GLMHelpers.h +++ b/libraries/shared/src/GLMHelpers.h @@ -229,8 +229,8 @@ void generateBasisVectors(const glm::vec3& primaryAxis, const glm::vec3& seconda glm::vec2 getFacingDir2D(const glm::quat& rot); glm::vec2 getFacingDir2D(const glm::mat4& m); -bool isNaN(glm::vec3 value); -bool isNaN(glm::quat value); +inline bool isNaN(const glm::vec3& value) { return isNaN(value.x) || isNaN(value.y) || isNaN(value.z); } +inline bool isNaN(const glm::quat& value) { return isNaN(value.w) || isNaN(value.x) || isNaN(value.y) || isNaN(value.z); } glm::mat4 orthoInverse(const glm::mat4& m); diff --git a/libraries/shared/src/SharedUtil.cpp b/libraries/shared/src/SharedUtil.cpp index bdc5d4c60d..8f68e20222 100644 --- a/libraries/shared/src/SharedUtil.cpp +++ b/libraries/shared/src/SharedUtil.cpp @@ -247,12 +247,6 @@ int getNthBit(unsigned char byte, int ordinal) { return ERROR_RESULT; } -bool isBetween(int64_t value, int64_t max, int64_t min) { - return ((value <= max) && (value >= min)); -} - - - void setSemiNibbleAt(unsigned char& byte, int bitIndex, int value) { //assert(value <= 3 && value >= 0); byte |= ((value & 3) << (6 - bitIndex)); // semi-nibbles store 00, 01, 10, or 11 @@ -260,12 +254,7 @@ void setSemiNibbleAt(unsigned char& byte, int bitIndex, int value) { bool isInEnvironment(const char* environment) { char* environmentString = getenv("HIFI_ENVIRONMENT"); - - if (environmentString && strcmp(environmentString, environment) == 0) { - return true; - } else { - return false; - } + return (environmentString && strcmp(environmentString, environment) == 0); } ////////////////////////////////////////////////////////////////////////////////////////// @@ -632,10 +621,6 @@ void debug::checkDeadBeef(void* memoryVoid, int size) { assert(memcmp((unsigned char*)memoryVoid, DEADBEEF, std::min(size, DEADBEEF_SIZE)) != 0); } -bool isNaN(float value) { - return value != value; -} - QString formatUsecTime(float usecs, int prec) { static const quint64 SECONDS_PER_MINUTE = 60; static const quint64 USECS_PER_MINUTE = USECS_PER_SECOND * SECONDS_PER_MINUTE; diff --git a/libraries/shared/src/SharedUtil.h b/libraries/shared/src/SharedUtil.h index 8fb65a5247..e9201b4a92 100644 --- a/libraries/shared/src/SharedUtil.h +++ b/libraries/shared/src/SharedUtil.h @@ -180,11 +180,11 @@ private: static int DEADBEEF_SIZE; }; -bool isBetween(int64_t value, int64_t max, int64_t min); - +/// \return true when value is between max and min +inline bool isBetween(int64_t value, int64_t max, int64_t min) { return ((value <= max) && (value >= min)); } /// \return bool is the float NaN -bool isNaN(float value); +inline bool isNaN(float value) { return value != value; } QString formatUsecTime(float usecs, int prec = 3); QString formatSecondsElapsed(float seconds); diff --git a/libraries/shared/src/Transform.cpp b/libraries/shared/src/Transform.cpp index a3a3c05731..c51b3dae4b 100644 --- a/libraries/shared/src/Transform.cpp +++ b/libraries/shared/src/Transform.cpp @@ -150,7 +150,3 @@ QJsonObject Transform::toJson(const Transform& transform) { } return result; } - -bool Transform::containsNaN() const { - return isNaN(_rotation) || isNaN(_scale) || isNaN(_translation); -} diff --git a/libraries/shared/src/Transform.h b/libraries/shared/src/Transform.h index 1024173cbd..1e1d10c54b 100644 --- a/libraries/shared/src/Transform.h +++ b/libraries/shared/src/Transform.h @@ -145,7 +145,7 @@ public: Vec4 transform(const Vec4& pos) const; Vec3 transform(const Vec3& pos) const; - bool containsNaN() const; + bool containsNaN() const { return isNaN(_rotation) || isNaN(glm::dot(_scale, _translation)); } protected: From bc0b3b38b3f763c1831340b6c73015a5aa5b6555 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 14 Apr 2016 12:25:47 -0700 Subject: [PATCH 33/40] remove narrowing conversion for ICE_SERVER_DEFAULT_PORT --- libraries/networking/src/NetworkPeer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/NetworkPeer.h b/libraries/networking/src/NetworkPeer.h index 0011f3da76..8298a2dad4 100644 --- a/libraries/networking/src/NetworkPeer.h +++ b/libraries/networking/src/NetworkPeer.h @@ -21,7 +21,7 @@ #include "HifiSockAddr.h" const QString ICE_SERVER_HOSTNAME = "localhost"; -const int ICE_SERVER_DEFAULT_PORT = 7337; +const quint16 ICE_SERVER_DEFAULT_PORT = 7337; const int ICE_HEARBEAT_INTERVAL_MSECS = 2 * 1000; const int MAX_ICE_CONNECTION_ATTEMPTS = 5; From 7e02b302276c078b49b6b2e094932809f5493ea0 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 14 Apr 2016 13:01:47 -0700 Subject: [PATCH 34/40] rename .app on OS X to Interface --- cmake/macros/SetPackagingParameters.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/macros/SetPackagingParameters.cmake b/cmake/macros/SetPackagingParameters.cmake index 63e7a1a174..978fa29552 100644 --- a/cmake/macros/SetPackagingParameters.cmake +++ b/cmake/macros/SetPackagingParameters.cmake @@ -25,14 +25,14 @@ macro(SET_PACKAGING_PARAMETERS) set(BUILD_VERSION ${RELEASE_NUMBER}) set(BUILD_ORGANIZATION "High Fidelity") set(HIGH_FIDELITY_PROTOCOL "hifi") - set(INTERFACE_BUNDLE_NAME "High Fidelity") + set(INTERFACE_BUNDLE_NAME "Interface") set(INTERFACE_ICON_PREFIX "interface") elseif (RELEASE_TYPE STREQUAL "PR") set(DEPLOY_PACKAGE TRUE) set(PR_BUILD 1) set(BUILD_VERSION "PR${RELEASE_NUMBER}") set(BUILD_ORGANIZATION "High Fidelity - ${BUILD_VERSION}") - set(INTERFACE_BUNDLE_NAME "High Fidelity") + set(INTERFACE_BUNDLE_NAME "Interface") set(INTERFACE_ICON_PREFIX "interface-beta") else () set(DEV_BUILD 1) From 757c7dcf29ba58154efaea546155a0bea36e2272 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 14 Apr 2016 13:40:13 -0700 Subject: [PATCH 35/40] removing HTTP health check from ice-server --- ice-server/src/IceServer.cpp | 28 +--------------------------- ice-server/src/IceServer.h | 7 +------ 2 files changed, 2 insertions(+), 33 deletions(-) diff --git a/ice-server/src/IceServer.cpp b/ice-server/src/IceServer.cpp index c3e57b1d36..537742ad8b 100644 --- a/ice-server/src/IceServer.cpp +++ b/ice-server/src/IceServer.cpp @@ -33,9 +33,7 @@ IceServer::IceServer(int argc, char* argv[]) : QCoreApplication(argc, argv), _id(QUuid::createUuid()), _serverSocket(), - _activePeers(), - _httpManager(QHostAddress::AnyIPv4, ICE_SERVER_MONITORING_PORT, QString("%1/web/").arg(QCoreApplication::applicationDirPath()), this), - _lastInactiveCheckTimestamp(QDateTime::currentMSecsSinceEpoch()) + _activePeers() { // start the ice-server socket qDebug() << "ice-server socket is listening on" << ICE_SERVER_DEFAULT_PORT; @@ -287,8 +285,6 @@ void IceServer::sendPeerInformationPacket(const NetworkPeer& peer, const HifiSoc void IceServer::clearInactivePeers() { NetworkPeerHash::iterator peerItem = _activePeers.begin(); - _lastInactiveCheckTimestamp = QDateTime::currentMSecsSinceEpoch(); - while (peerItem != _activePeers.end()) { SharedNetworkPeer peer = peerItem.value(); @@ -306,25 +302,3 @@ void IceServer::clearInactivePeers() { } } } - -bool IceServer::handleHTTPRequest(HTTPConnection* connection, const QUrl& url, bool skipSubHandler) { - // We need an HTTP handler in order to monitor the health of the ice server - // The correct functioning of the ICE server will be determined by its HTTP availability, - - if (connection->requestOperation() == QNetworkAccessManager::GetOperation) { - if (url.path() == "/status") { - // figure out if we respond with 0 (we're good) or 1 (we think we're in trouble) - - const quint64 MAX_PACKET_GAP_MS_FOR_STUCK_SOCKET = 10 * 1000; - - auto sinceLastInactiveCheck = QDateTime::currentMSecsSinceEpoch() - _lastInactiveCheckTimestamp; - int statusNumber = (sinceLastInactiveCheck > MAX_PACKET_GAP_MS_FOR_STUCK_SOCKET) ? 1 : 0; - - connection->respond(HTTPConnection::StatusCode200, QByteArray::number(statusNumber)); - - return true; - } - } - - return false; -} diff --git a/ice-server/src/IceServer.h b/ice-server/src/IceServer.h index 7d1d05324c..a204c59e28 100644 --- a/ice-server/src/IceServer.h +++ b/ice-server/src/IceServer.h @@ -28,11 +28,10 @@ class QNetworkReply; -class IceServer : public QCoreApplication, public HTTPRequestHandler { +class IceServer : public QCoreApplication { Q_OBJECT public: IceServer(int argc, char* argv[]); - bool handleHTTPRequest(HTTPConnection* connection, const QUrl& url, bool skipSubHandler = false); private slots: void clearInactivePeers(); void publicKeyReplyFinished(QNetworkReply* reply); @@ -52,13 +51,9 @@ private: using NetworkPeerHash = QHash; NetworkPeerHash _activePeers; - HTTPManager _httpManager; - using RSAUniquePtr = std::unique_ptr>; using DomainPublicKeyHash = std::unordered_map; DomainPublicKeyHash _domainPublicKeys; - - quint64 _lastInactiveCheckTimestamp; }; #endif // hifi_IceServer_h From 7385894b668a116d2fc3286d0c22d3294ff05225 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 14 Apr 2016 13:57:09 -0700 Subject: [PATCH 36/40] keep a set of pending public key requests --- ice-server/src/IceServer.cpp | 61 +++++++++++++++++++++--------------- ice-server/src/IceServer.h | 2 ++ 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/ice-server/src/IceServer.cpp b/ice-server/src/IceServer.cpp index 537742ad8b..df8afdb82f 100644 --- a/ice-server/src/IceServer.cpp +++ b/ice-server/src/IceServer.cpp @@ -167,39 +167,42 @@ SharedNetworkPeer IceServer::addOrUpdateHeartbeatingPeer(NLPacket& packet) { } bool IceServer::isVerifiedHeartbeat(const QUuid& domainID, const QByteArray& plaintext, const QByteArray& signature) { - // check if we have a public key for this domain ID - if we do not then fire off the request for it - auto it = _domainPublicKeys.find(domainID); - if (it != _domainPublicKeys.end()) { + // make sure we're not already waiting for a public key for this domain-server + if (!_pendingPublicKeyRequests.contains(domainID)) { + // check if we have a public key for this domain ID - if we do not then fire off the request for it + auto it = _domainPublicKeys.find(domainID); + if (it != _domainPublicKeys.end()) { - // attempt to verify the signature for this heartbeat - const auto rsaPublicKey = it->second.get(); + // attempt to verify the signature for this heartbeat + const auto rsaPublicKey = it->second.get(); - if (rsaPublicKey) { - auto hashedPlaintext = QCryptographicHash::hash(plaintext, QCryptographicHash::Sha256); - int verificationResult = RSA_verify(NID_sha256, - reinterpret_cast(hashedPlaintext.constData()), - hashedPlaintext.size(), - reinterpret_cast(signature.constData()), - signature.size(), - rsaPublicKey); + if (rsaPublicKey) { + auto hashedPlaintext = QCryptographicHash::hash(plaintext, QCryptographicHash::Sha256); + int verificationResult = RSA_verify(NID_sha256, + reinterpret_cast(hashedPlaintext.constData()), + hashedPlaintext.size(), + reinterpret_cast(signature.constData()), + signature.size(), + rsaPublicKey); + + if (verificationResult == 1) { + // this is the only success case - we return true here to indicate that the heartbeat is verified + return true; + } else { + qDebug() << "Failed to verify heartbeat for" << domainID << "- re-requesting public key from API."; + } - if (verificationResult == 1) { - // this is the only success case - we return true here to indicate that the heartbeat is verified - return true; } else { - qDebug() << "Failed to verify heartbeat for" << domainID << "- re-requesting public key from API."; + // we can't let this user in since we couldn't convert their public key to an RSA key we could use + qWarning() << "Public key for" << domainID << "is not a usable RSA* public key."; + qWarning() << "Re-requesting public key from API"; } - - } else { - // we can't let this user in since we couldn't convert their public key to an RSA key we could use - qWarning() << "Public key for" << domainID << "is not a usable RSA* public key."; - qWarning() << "Re-requesting public key from API"; } - } - // we could not verify this heartbeat (missing public key, could not load public key, bad actor) - // ask the metaverse API for the right public key and return false to indicate that this is not verified - requestDomainPublicKey(domainID); + // we could not verify this heartbeat (missing public key, could not load public key, bad actor) + // ask the metaverse API for the right public key and return false to indicate that this is not verified + requestDomainPublicKey(domainID); + } return false; } @@ -217,6 +220,9 @@ void IceServer::requestDomainPublicKey(const QUuid& domainID) { qDebug() << "Requesting public key for domain with ID" << domainID; + // add this to the set of pending public key requests + _pendingPublicKeyRequests.insert(domainID); + networkAccessManager.get(publicKeyRequest); } @@ -269,6 +275,9 @@ void IceServer::publicKeyReplyFinished(QNetworkReply* reply) { qWarning() << "Error retreiving public key for domain with ID" << domainID << "-" << reply->errorString(); } + // remove this domain ID from the list of pending public key requests + _pendingPublicKeyRequests.remove(domainID); + reply->deleteLater(); } diff --git a/ice-server/src/IceServer.h b/ice-server/src/IceServer.h index a204c59e28..2aa9a875a7 100644 --- a/ice-server/src/IceServer.h +++ b/ice-server/src/IceServer.h @@ -54,6 +54,8 @@ private: using RSAUniquePtr = std::unique_ptr>; using DomainPublicKeyHash = std::unordered_map; DomainPublicKeyHash _domainPublicKeys; + + QSet _pendingPublicKeyRequests; }; #endif // hifi_IceServer_h From 9bca2309e098a849c58eb3f4cf876127c45426c2 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 14 Apr 2016 13:58:02 -0700 Subject: [PATCH 37/40] remove the monitoring endpoint debug --- ice-server/src/IceServer.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/ice-server/src/IceServer.cpp b/ice-server/src/IceServer.cpp index df8afdb82f..84c48ac5f9 100644 --- a/ice-server/src/IceServer.cpp +++ b/ice-server/src/IceServer.cpp @@ -27,8 +27,6 @@ const int CLEAR_INACTIVE_PEERS_INTERVAL_MSECS = 1 * 1000; const int PEER_SILENCE_THRESHOLD_MSECS = 5 * 1000; -const quint16 ICE_SERVER_MONITORING_PORT = 40110; - IceServer::IceServer(int argc, char* argv[]) : QCoreApplication(argc, argv), _id(QUuid::createUuid()), @@ -37,7 +35,6 @@ IceServer::IceServer(int argc, char* argv[]) : { // start the ice-server socket qDebug() << "ice-server socket is listening on" << ICE_SERVER_DEFAULT_PORT; - qDebug() << "monitoring http endpoint is listening on " << ICE_SERVER_MONITORING_PORT; _serverSocket.bind(QHostAddress::AnyIPv4, ICE_SERVER_DEFAULT_PORT); // set processPacket as the verified packet callback for the udt::Socket From 84df6cfed02bb60908c95c7bc4c002bc0764293e Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 14 Apr 2016 14:46:08 -0700 Subject: [PATCH 38/40] only re-request ice addresses during failure --- domain-server/src/DomainServer.cpp | 22 +++++++++------------- domain-server/src/DomainServer.h | 3 +-- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 5df9492f87..a5c485ebe2 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -489,15 +489,6 @@ void DomainServer::setupICEHeartbeatForFullNetworking() { // lookup the available ice-server hosts now updateICEServerAddresses(); - const int ICE_ADDRESS_UPDATE_MSECS = 30 * 1000; - - // hookup a timer to keep those updated every ICE_ADDRESS_UPDATE_MSECS in case of a failure requiring a switchover - if (_iceAddressLookupTimer) { - _iceAddressLookupTimer = new QTimer { this }; - connect(_iceAddressLookupTimer, &QTimer::timeout, this, &DomainServer::updateICEServerAddresses); - _iceAddressLookupTimer->start(ICE_ADDRESS_UPDATE_MSECS); - } - // call our sendHeartbeatToIceServer immediately anytime a local or public socket changes connect(limitedNodeList.data(), &LimitedNodeList::localSockAddrChanged, this, &DomainServer::sendHeartbeatToIceServer); @@ -1170,7 +1161,8 @@ void DomainServer::sendHeartbeatToIceServer() { // reset the connection flag for ICE server _connectedToICEServer = false; - randomizeICEServerAddress(); + // randomize our ice-server address (and simultaneously look up any new hostnames for available ice-servers) + randomizeICEServerAddress(true); } // NOTE: I'd love to specify the correct size for the packet here, but it's a little trickey with @@ -2164,13 +2156,17 @@ void DomainServer::handleICEHostInfo(const QHostInfo& hostInfo) { } if (_iceServerSocket.isNull()) { - // we don't have a candidate ice-server yet, pick now - randomizeICEServerAddress(); + // we don't have a candidate ice-server yet, pick now (without triggering a host lookup since we just did one) + randomizeICEServerAddress(false); } } } -void DomainServer::randomizeICEServerAddress() { +void DomainServer::randomizeICEServerAddress(bool shouldTriggerHostLookup) { + if (shouldTriggerHostLookup) { + updateICEServerAddresses(); + } + // create a list by removing the already failed ice-server addresses auto candidateICEAddresses = _iceServerAddresses; diff --git a/domain-server/src/DomainServer.h b/domain-server/src/DomainServer.h index 1ece2e30dd..93bb5de494 100644 --- a/domain-server/src/DomainServer.h +++ b/domain-server/src/DomainServer.h @@ -105,7 +105,7 @@ private: void setupICEHeartbeatForFullNetworking(); void sendHeartbeatToDataServer(const QString& networkAddress); - void randomizeICEServerAddress(); + void randomizeICEServerAddress(bool shouldTriggerHostLookup); unsigned int countConnectedUsers(); @@ -172,7 +172,6 @@ private: QList _iceServerAddresses; QSet _failedIceServerAddresses; - QTimer* _iceAddressLookupTimer { nullptr }; // this looks like a dangling pointer but is parented to the DomainServer int _iceAddressLookupID { -1 }; int _noReplyICEHeartbeats { 0 }; int _numHeartbeatDenials { 0 }; From bb02af793c410f25d1c5932baf57c98733edcce9 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 14 Apr 2016 13:14:25 -0700 Subject: [PATCH 39/40] Name exposed cache objects --- libraries/animation/src/AnimationCache.cpp | 1 + libraries/audio/src/SoundCache.cpp | 1 + libraries/model-networking/src/model-networking/ModelCache.cpp | 1 + libraries/model-networking/src/model-networking/TextureCache.cpp | 1 + 4 files changed, 4 insertions(+) diff --git a/libraries/animation/src/AnimationCache.cpp b/libraries/animation/src/AnimationCache.cpp index 02d3f8873e..a502e608e2 100644 --- a/libraries/animation/src/AnimationCache.cpp +++ b/libraries/animation/src/AnimationCache.cpp @@ -22,6 +22,7 @@ AnimationCache::AnimationCache(QObject* parent) : { const qint64 ANIMATION_DEFAULT_UNUSED_MAX_SIZE = 50 * BYTES_PER_MEGABYTES; setUnusedResourceCacheSize(ANIMATION_DEFAULT_UNUSED_MAX_SIZE); + setObjectName("AnimationCache"); } AnimationPointer AnimationCache::getAnimation(const QUrl& url) { diff --git a/libraries/audio/src/SoundCache.cpp b/libraries/audio/src/SoundCache.cpp index abcdb2da7c..2752c6669f 100644 --- a/libraries/audio/src/SoundCache.cpp +++ b/libraries/audio/src/SoundCache.cpp @@ -21,6 +21,7 @@ SoundCache::SoundCache(QObject* parent) : { const qint64 SOUND_DEFAULT_UNUSED_MAX_SIZE = 50 * BYTES_PER_MEGABYTES; setUnusedResourceCacheSize(SOUND_DEFAULT_UNUSED_MAX_SIZE); + setObjectName("SoundCache"); } SharedSoundPointer SoundCache::getSound(const QUrl& url) { diff --git a/libraries/model-networking/src/model-networking/ModelCache.cpp b/libraries/model-networking/src/model-networking/ModelCache.cpp index b8deef9f27..68878385a8 100644 --- a/libraries/model-networking/src/model-networking/ModelCache.cpp +++ b/libraries/model-networking/src/model-networking/ModelCache.cpp @@ -226,6 +226,7 @@ void GeometryDefinitionResource::setGeometryDefinition(void* fbxGeometry) { ModelCache::ModelCache() { const qint64 GEOMETRY_DEFAULT_UNUSED_MAX_SIZE = DEFAULT_UNUSED_MAX_SIZE; setUnusedResourceCacheSize(GEOMETRY_DEFAULT_UNUSED_MAX_SIZE); + setObjectName("ModelCache"); } QSharedPointer ModelCache::createResource(const QUrl& url, const QSharedPointer& fallback, diff --git a/libraries/model-networking/src/model-networking/TextureCache.cpp b/libraries/model-networking/src/model-networking/TextureCache.cpp index f314f6cb06..e482c20b11 100644 --- a/libraries/model-networking/src/model-networking/TextureCache.cpp +++ b/libraries/model-networking/src/model-networking/TextureCache.cpp @@ -34,6 +34,7 @@ TextureCache::TextureCache() { const qint64 TEXTURE_DEFAULT_UNUSED_MAX_SIZE = DEFAULT_UNUSED_MAX_SIZE; setUnusedResourceCacheSize(TEXTURE_DEFAULT_UNUSED_MAX_SIZE); + setObjectName("TextureCache"); } TextureCache::~TextureCache() { From 4ed8573f74479fe0da71c18676417cc1266f0e83 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 14 Apr 2016 17:28:24 -0700 Subject: [PATCH 40/40] CR --- interface/src/scripting/GlobalServicesScriptingInterface.cpp | 2 +- interface/src/ui/Stats.cpp | 2 +- libraries/networking/src/ResourceCache.cpp | 2 +- libraries/networking/src/ResourceCache.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/interface/src/scripting/GlobalServicesScriptingInterface.cpp b/interface/src/scripting/GlobalServicesScriptingInterface.cpp index e6d87a0260..e8d63a6d99 100644 --- a/interface/src/scripting/GlobalServicesScriptingInterface.cpp +++ b/interface/src/scripting/GlobalServicesScriptingInterface.cpp @@ -122,7 +122,7 @@ void DownloadInfoResultFromScriptValue(const QScriptValue& object, DownloadInfoR DownloadInfoResult GlobalServicesScriptingInterface::getDownloadInfo() { DownloadInfoResult result; - foreach(auto resource, ResourceCache::getLoadingRequests()) { + foreach(const auto& resource, ResourceCache::getLoadingRequests()) { result.downloading.append(resource->getProgress() * 100.0f); } result.pending = ResourceCache::getPendingRequestCount(); diff --git a/interface/src/ui/Stats.cpp b/interface/src/ui/Stats.cpp index d1cb4cf3c2..ec4b2280b6 100644 --- a/interface/src/ui/Stats.cpp +++ b/interface/src/ui/Stats.cpp @@ -214,7 +214,7 @@ void Stats::updateStats(bool force) { // If the urls have changed, update the list if (shouldUpdateUrls) { _downloadUrls.clear(); - foreach (auto resource, loadingRequests) { + foreach (const auto& resource, loadingRequests) { _downloadUrls << resource->getURL().toString(); } emit downloadUrlsChanged(); diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index 5f7cf7926a..aa8022e89b 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -295,7 +295,7 @@ bool ResourceCache::attemptRequest(QSharedPointer resource) { return true; } -void ResourceCache::requestCompleted(QSharedPointer resource) { +void ResourceCache::requestCompleted(QWeakPointer resource) { auto sharedItems = DependencyManager::get(); sharedItems->removeRequest(resource); --_requestsActive; diff --git a/libraries/networking/src/ResourceCache.h b/libraries/networking/src/ResourceCache.h index 94fcafee9e..ad9ba10a7c 100644 --- a/libraries/networking/src/ResourceCache.h +++ b/libraries/networking/src/ResourceCache.h @@ -141,7 +141,7 @@ protected: /// Attempt to load a resource if requests are below the limit, otherwise queue the resource for loading /// \return true if the resource began loading, otherwise false if the resource is in the pending queue static bool attemptRequest(QSharedPointer resource); - static void requestCompleted(QSharedPointer resource); + static void requestCompleted(QWeakPointer resource); static bool attemptHighestPriorityRequest(); private: