From 80c0f2a21e6f549fff129a460d80b15a3f47d48f Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Fri, 2 Feb 2018 11:28:25 -0800 Subject: [PATCH 1/4] Fix crash when passing --checkMinSpec flag That flag caused a DLL to be loaded before Application was instanced. This triggers a Qt bug inside Q_COREAPP_STARTUP_FUNC that causes the previous registration pointing the startup function in the main executable to be overridden with the address of the function in the DLL (Since they both link the same static library) This leads to the correct function running in the wrong address space (the DLLs), hence not initializing some global variables correctly. --- assignment-client/src/main.cpp | 3 + domain-server/src/main.cpp | 2 + interface/src/main.cpp | 2 + libraries/shared/src/SettingInterface.cpp | 69 ++++++++++++++--------- libraries/shared/src/SettingManager.cpp | 6 +- libraries/shared/src/SharedUtil.cpp | 9 ++- libraries/shared/src/SharedUtil.h | 34 +---------- tools/ac-client/src/main.cpp | 5 +- tools/atp-client/src/main.cpp | 3 +- tools/oven/src/main.cpp | 8 ++- 10 files changed, 68 insertions(+), 73 deletions(-) diff --git a/assignment-client/src/main.cpp b/assignment-client/src/main.cpp index 4f64bf8f7f..bf4497984f 100644 --- a/assignment-client/src/main.cpp +++ b/assignment-client/src/main.cpp @@ -23,9 +23,12 @@ int main(int argc, char* argv[]) { QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); QCoreApplication::setApplicationVersion(BuildInfo::VERSION); + setupGlobalInstances(); + qInstallMessageHandler(LogHandler::verboseMessageHandler); qInfo() << "Starting."; + AssignmentClientApp app(argc, argv); int acReturn = app.exec(); diff --git a/domain-server/src/main.cpp b/domain-server/src/main.cpp index dc3ee54fe7..e258626223 100644 --- a/domain-server/src/main.cpp +++ b/domain-server/src/main.cpp @@ -29,6 +29,8 @@ int main(int argc, char* argv[]) { QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); QCoreApplication::setApplicationVersion(BuildInfo::VERSION); + setupGlobalInstances(); + Setting::init(); #ifndef WIN32 diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 30e8439985..0f8134b253 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -62,6 +62,8 @@ int main(int argc, const char* argv[]) { QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); QCoreApplication::setApplicationVersion(BuildInfo::VERSION); + setupGlobalInstances(); + Setting::init(); // Instance UserActivityLogger now that the settings are loaded diff --git a/libraries/shared/src/SettingInterface.cpp b/libraries/shared/src/SettingInterface.cpp index 327668574e..62f116795e 100644 --- a/libraries/shared/src/SettingInterface.cpp +++ b/libraries/shared/src/SettingInterface.cpp @@ -23,43 +23,53 @@ #include "SharedUtil.h" namespace Setting { - static QSharedPointer globalManager; - // cleans up the settings private instance. Should only be run once at closing down. void cleanupPrivateInstance() { + auto globalManager = DependencyManager::get(); + Q_ASSERT(qApp && globalManager); + // grab the thread before we nuke the instance - QThread* settingsManagerThread = DependencyManager::get()->thread(); + QThread* settingsManagerThread = globalManager->thread(); - // tell the private instance to clean itself up on its thread - DependencyManager::destroy(); - - globalManager.reset(); - - // quit the settings manager thread and wait on it to make sure it's gone + // quit the settings manager thread settingsManagerThread->quit(); settingsManagerThread->wait(); + + // Save all settings + globalManager->saveAll(); + + qCDebug(shared) << "Settings thread stopped."; } void setupPrivateInstance() { - // Ensure Setting::init has already ran and qApp exists - if (qApp && globalManager) { - // Let's set up the settings Private instance on its own thread - QThread* thread = new QThread(); - Q_CHECK_PTR(thread); - thread->setObjectName("Settings Thread"); + auto globalManager = DependencyManager::get(); + Q_ASSERT(qApp && globalManager); - QObject::connect(thread, SIGNAL(started()), globalManager.data(), SLOT(startTimer())); - QObject::connect(thread, SIGNAL(finished()), thread, SLOT(deleteLater())); - QObject::connect(thread, SIGNAL(finished()), globalManager.data(), SLOT(deleteLater())); - globalManager->moveToThread(thread); - thread->start(); - qCDebug(shared) << "Settings thread started."; + // Let's set up the settings private instance on its own thread + QThread* thread = new QThread(qApp); + Q_CHECK_PTR(thread); + thread->setObjectName("Settings Thread"); - // Register cleanupPrivateInstance to run inside QCoreApplication's destructor. - qAddPostRoutine(cleanupPrivateInstance); - } + // Setup setting periodical save timer + QObject::connect(thread, &QThread::started, globalManager.data(), &Manager::startTimer); + QObject::connect(thread, &QThread::finished, globalManager.data(), &Manager::stopTimer); + + // Setup manager threading affinity + globalManager->moveToThread(thread); + QObject::connect(thread, &QThread::finished, globalManager.data(), [] { + auto globalManager = DependencyManager::get(); + Q_ASSERT(qApp && globalManager); + + // Move manager back to the main thread (has to be done on owning thread) + globalManager->moveToThread(qApp->thread()); + }); + + thread->start(); + qCDebug(shared) << "Settings thread started."; + + // Register cleanupPrivateInstance to run inside QCoreApplication's destructor. + qAddPostRoutine(cleanupPrivateInstance); } - FIXED_Q_COREAPP_STARTUP_FUNCTION(setupPrivateInstance) // Sets up the settings private instance. Should only be run once at startup. preInit() must be run beforehand, void init() { @@ -68,6 +78,7 @@ namespace Setting { QSettings settings; qCDebug(shared) << "Settings file:" << settings.fileName(); + // Backward compatibility for old settings file if (settings.allKeys().isEmpty()) { loadOldINIFile(settings); } @@ -80,11 +91,13 @@ namespace Setting { qCDebug(shared) << (deleted ? "Deleted" : "Failed to delete") << "settings lock file" << settingsLockFilename; } - globalManager = DependencyManager::set(); + // Setup settings manager + DependencyManager::set(); - setupPrivateInstance(); + // Add pre-routine to setup threading + qAddPreRoutine(setupPrivateInstance); } - + void Interface::init() { if (!DependencyManager::isSet()) { // WARNING: As long as we are using QSettings this should always be triggered for each Setting::Handle diff --git a/libraries/shared/src/SettingManager.cpp b/libraries/shared/src/SettingManager.cpp index 6c246d4cea..2e0850255a 100644 --- a/libraries/shared/src/SettingManager.cpp +++ b/libraries/shared/src/SettingManager.cpp @@ -23,11 +23,7 @@ namespace Setting { // Cleanup timer stopTimer(); delete _saveTimer; - - // Save all settings before exit - saveAll(); - - // sync will be called in the QSettings destructor + _saveTimer = nullptr; } // Custom deleter does nothing, because we need to shutdown later than the dependency manager diff --git a/libraries/shared/src/SharedUtil.cpp b/libraries/shared/src/SharedUtil.cpp index 8e5c30711c..772340b631 100644 --- a/libraries/shared/src/SharedUtil.cpp +++ b/libraries/shared/src/SharedUtil.cpp @@ -63,12 +63,12 @@ extern "C" FILE * __cdecl __iob_func(void) { #include "OctalCode.h" #include "SharedLogging.h" +static std::mutex stagedGlobalInstancesMutex; static std::unordered_map stagedGlobalInstances; std::mutex& globalInstancesMutex() { - static std::mutex mutex; - return mutex; + return stagedGlobalInstancesMutex; } static void commitGlobalInstances() { @@ -78,7 +78,10 @@ static void commitGlobalInstances() { } stagedGlobalInstances.clear(); } -FIXED_Q_COREAPP_STARTUP_FUNCTION(commitGlobalInstances) + +void setupGlobalInstances() { + qAddPreRoutine(commitGlobalInstances); +} QVariant getGlobalInstance(const char* propertyName) { if (qApp) { diff --git a/libraries/shared/src/SharedUtil.h b/libraries/shared/src/SharedUtil.h index 5a1e48d9c0..64bbc6585b 100644 --- a/libraries/shared/src/SharedUtil.h +++ b/libraries/shared/src/SharedUtil.h @@ -25,23 +25,6 @@ #include #include -// Workaround for https://bugreports.qt.io/browse/QTBUG-54479 -// Wrap target function inside another function that holds -// a unique string identifier and uses it to ensure it only runs once -// by storing a state within the qApp -// We cannot used std::call_once with a static once_flag because -// this is used in shared libraries that are linked by several DLLs -// (ie. plugins), meaning the static will be useless in that case -#define FIXED_Q_COREAPP_STARTUP_FUNCTION(AFUNC) \ - static void AFUNC ## _fixed() { \ - const auto propertyName = std::string(Q_FUNC_INFO) + __FILE__; \ - if (!qApp->property(propertyName.c_str()).toBool()) { \ - AFUNC(); \ - qApp->setProperty(propertyName.c_str(), QVariant(true)); \ - } \ - } \ - Q_COREAPP_STARTUP_FUNCTION(AFUNC ## _fixed) - // When writing out avatarEntities to a QByteArray, if the parentID is the ID of MyAvatar, use this ID instead. This allows // the value to be reset when the sessionID changes. const QUuid AVATAR_SELF_ID = QUuid("{00000000-0000-0000-0000-000000000001}"); @@ -53,21 +36,7 @@ std::unique_ptr& globalInstancePointer() { return instancePtr; } -template -void setGlobalInstance(const char* propertyName, T* instance) { - globalInstancePointer().reset(instance); -} - -template -bool destroyGlobalInstance() { - std::unique_ptr& instancePtr = globalInstancePointer(); - if (instancePtr.get()) { - instancePtr.reset(); - return true; - } - return false; -} - +void setupGlobalInstances(); std::mutex& globalInstancesMutex(); QVariant getGlobalInstance(const char* propertyName); void setGlobalInstance(const char* propertyName, const QVariant& variant); @@ -78,7 +47,6 @@ void setGlobalInstance(const char* propertyName, const QVariant& variant); template T* globalInstance(const char* propertyName, Args&&... args) { static T* resultInstance { nullptr }; - static std::mutex mutex; if (!resultInstance) { std::unique_lock lock(globalInstancesMutex()); if (!resultInstance) { diff --git a/tools/ac-client/src/main.cpp b/tools/ac-client/src/main.cpp index c9affde3b5..139e44bc9a 100644 --- a/tools/ac-client/src/main.cpp +++ b/tools/ac-client/src/main.cpp @@ -19,15 +19,16 @@ using namespace std; -int main(int argc, char * argv[]) { +int main(int argc, char* argv[]) { QCoreApplication::setApplicationName(BuildInfo::AC_CLIENT_SERVER_NAME); QCoreApplication::setOrganizationName(BuildInfo::MODIFIED_ORGANIZATION); QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); QCoreApplication::setApplicationVersion(BuildInfo::VERSION); + setupGlobalInstances(); + Setting::init(); ACClientApp app(argc, argv); - return app.exec(); } diff --git a/tools/atp-client/src/main.cpp b/tools/atp-client/src/main.cpp index 830c049bc7..0a8274fedd 100644 --- a/tools/atp-client/src/main.cpp +++ b/tools/atp-client/src/main.cpp @@ -25,9 +25,10 @@ int main(int argc, char * argv[]) { QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); QCoreApplication::setApplicationVersion(BuildInfo::VERSION); + setupGlobalInstances(); + Setting::init(); ATPClientApp app(argc, argv); - return app.exec(); } diff --git a/tools/oven/src/main.cpp b/tools/oven/src/main.cpp index 788470b75e..be0c22a0c6 100644 --- a/tools/oven/src/main.cpp +++ b/tools/oven/src/main.cpp @@ -10,11 +10,17 @@ #include "Oven.h" +#include #include +#include int main (int argc, char** argv) { - QCoreApplication::setOrganizationName("High Fidelity"); QCoreApplication::setApplicationName("Oven"); + QCoreApplication::setOrganizationName(BuildInfo::MODIFIED_ORGANIZATION); + QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); + QCoreApplication::setApplicationVersion(BuildInfo::VERSION); + + setupGlobalInstances(); // init the settings interface so we can save and load settings Setting::init(); From 277e556b48666cddee5e04ac181caa1cd969fa8f Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 22 Feb 2018 16:22:18 -0800 Subject: [PATCH 2/4] Create a standard function to init Hifi Apps --- assignment-client/src/main.cpp | 16 ++-------------- domain-server/src/main.cpp | 16 +--------------- ice-server/src/main.cpp | 11 +++-------- interface/src/Application.cpp | 7 ++----- interface/src/main.cpp | 11 +---------- libraries/shared/src/LogHandler.cpp | 12 ++++++++++-- libraries/shared/src/SharedUtil.cpp | 20 ++++++++++++++++++++ libraries/shared/src/SharedUtil.h | 1 + tests/entities/src/main.cpp | 3 +++ tests/gpu-test/src/main.cpp | 5 ++++- tests/ktx/src/KtxTests.cpp | 3 ++- tests/qml/src/main.cpp | 12 ++---------- tests/qt59/src/main.cpp | 7 ++----- tests/recording/src/main.cpp | 16 +++++----------- tests/render-perf/src/main.cpp | 18 ++---------------- tests/render-texture-load/src/main.cpp | 16 ++-------------- tests/render-utils/src/main.cpp | 17 +++-------------- tests/shaders/src/main.cpp | 16 +++------------- tools/ac-client/src/main.cpp | 7 +------ tools/atp-client/src/main.cpp | 7 +------ tools/auto-tester/src/main.cpp | 2 +- tools/ice-client/src/main.cpp | 4 ++++ tools/oven/src/main.cpp | 7 +------ tools/skeleton-dump/src/main.cpp | 4 ++++ tools/udt-test/src/UDTTest.cpp | 2 -- tools/udt-test/src/main.cpp | 4 ++++ tools/vhacd-util/src/main.cpp | 4 ++++ 27 files changed, 88 insertions(+), 160 deletions(-) diff --git a/assignment-client/src/main.cpp b/assignment-client/src/main.cpp index bf4497984f..971e9ed272 100644 --- a/assignment-client/src/main.cpp +++ b/assignment-client/src/main.cpp @@ -9,25 +9,13 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#include +#include #include #include "AssignmentClientApp.h" -#include int main(int argc, char* argv[]) { - disableQtBearerPoll(); // Fixes wifi ping spikes - - QCoreApplication::setApplicationName(BuildInfo::ASSIGNMENT_CLIENT_NAME); - QCoreApplication::setOrganizationName(BuildInfo::MODIFIED_ORGANIZATION); - QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); - QCoreApplication::setApplicationVersion(BuildInfo::VERSION); - - setupGlobalInstances(); - - qInstallMessageHandler(LogHandler::verboseMessageHandler); - qInfo() << "Starting."; - + setupHifiApplication(BuildInfo::ASSIGNMENT_CLIENT_NAME); AssignmentClientApp app(argc, argv); diff --git a/domain-server/src/main.cpp b/domain-server/src/main.cpp index e258626223..d7856bf867 100644 --- a/domain-server/src/main.cpp +++ b/domain-server/src/main.cpp @@ -22,24 +22,10 @@ #include "DomainServer.h" int main(int argc, char* argv[]) { - disableQtBearerPoll(); // Fixes wifi ping spikes + setupHifiApplication(BuildInfo::DOMAIN_SERVER_NAME); - QCoreApplication::setApplicationName(BuildInfo::DOMAIN_SERVER_NAME); - QCoreApplication::setOrganizationName(BuildInfo::MODIFIED_ORGANIZATION); - QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); - QCoreApplication::setApplicationVersion(BuildInfo::VERSION); - - setupGlobalInstances(); - Setting::init(); -#ifndef WIN32 - setvbuf(stdout, NULL, _IOLBF, 0); -#endif - - qInstallMessageHandler(LogHandler::verboseMessageHandler); - qInfo() << "Starting."; - int currentExitCode = 0; // use a do-while to handle domain-server restart diff --git a/ice-server/src/main.cpp b/ice-server/src/main.cpp index ec8b9957cf..aac6cc0422 100644 --- a/ice-server/src/main.cpp +++ b/ice-server/src/main.cpp @@ -11,18 +11,13 @@ #include -#include +#include #include "IceServer.h" int main(int argc, char* argv[]) { -#ifndef WIN32 - setvbuf(stdout, NULL, _IOLBF, 0); -#endif - - qInstallMessageHandler(LogHandler::verboseMessageHandler); - qInfo() << "Starting."; + setupHifiApplication("Ice Server"); IceServer iceServer(argc, argv); return iceServer.exec(); -} \ No newline at end of file +} diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 3aa0f3d889..e956195ca6 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -575,10 +575,7 @@ void messageHandler(QtMsgType type, const QMessageLogContext& context, const QSt QString logMessage = LogHandler::getInstance().printMessage((LogMsgType) type, context, message); if (!logMessage.isEmpty()) { -#ifdef Q_OS_WIN - OutputDebugStringA(logMessage.toLocal8Bit().constData()); - OutputDebugStringA("\n"); -#elif defined Q_OS_ANDROID +#ifdef Q_OS_ANDROID const char * local=logMessage.toStdString().c_str(); switch (type) { case QtDebugMsg: @@ -599,7 +596,7 @@ void messageHandler(QtMsgType type, const QMessageLogContext& context, const QSt abort(); } #endif - qApp->getLogger()->addMessage(qPrintable(logMessage + "\n")); + qApp->getLogger()->addMessage(qPrintable(logMessage)); } } diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 0f8134b253..e80dc1d213 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -38,6 +38,7 @@ extern "C" { #endif int main(int argc, const char* argv[]) { + setupHifiApplication(BuildInfo::INTERFACE_NAME); #ifdef Q_OS_LINUX QApplication::setAttribute(Qt::AA_DontUseNativeMenuBar); @@ -51,19 +52,9 @@ int main(int argc, const char* argv[]) { QCoreApplication::setAttribute(Qt::AA_UseOpenGLES); #endif - disableQtBearerPoll(); // Fixes wifi ping spikes - QElapsedTimer startupTime; startupTime.start(); - // Set application infos - QCoreApplication::setApplicationName(BuildInfo::INTERFACE_NAME); - QCoreApplication::setOrganizationName(BuildInfo::MODIFIED_ORGANIZATION); - QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); - QCoreApplication::setApplicationVersion(BuildInfo::VERSION); - - setupGlobalInstances(); - Setting::init(); // Instance UserActivityLogger now that the settings are loaded diff --git a/libraries/shared/src/LogHandler.cpp b/libraries/shared/src/LogHandler.cpp index aa67c14c4b..ff0d68dcce 100644 --- a/libraries/shared/src/LogHandler.cpp +++ b/libraries/shared/src/LogHandler.cpp @@ -14,6 +14,10 @@ #include +#ifdef Q_OS_WIN +#include +#endif + #include #include #include @@ -184,8 +188,12 @@ QString LogHandler::printMessage(LogMsgType type, const QMessageLogContext& cont } } - QString logMessage = QString("%1 %2").arg(prefixString, message.split('\n').join('\n' + prefixString + " ")); - fprintf(stdout, "%s\n", qPrintable(logMessage)); + QString logMessage = QString("%1 %2\n").arg(prefixString, message.split('\n').join('\n' + prefixString + " ")); + + fprintf(stdout, "%s", qPrintable(logMessage)); +#ifdef Q_OS_WIN + OutputDebugStringA(qPrintable(logMessage)); +#endif return logMessage; } diff --git a/libraries/shared/src/SharedUtil.cpp b/libraries/shared/src/SharedUtil.cpp index 772340b631..d05e940322 100644 --- a/libraries/shared/src/SharedUtil.cpp +++ b/libraries/shared/src/SharedUtil.cpp @@ -59,6 +59,9 @@ extern "C" FILE * __cdecl __iob_func(void) { #include #include +#include + +#include "LogHandler.h" #include "NumericalConstants.h" #include "OctalCode.h" #include "SharedLogging.h" @@ -1179,6 +1182,23 @@ void watchParentProcess(int parentPID) { } #endif +void setupHifiApplication(QString applicationName) { + disableQtBearerPoll(); // Fixes wifi ping spikes + + QCoreApplication::setApplicationName(applicationName); + QCoreApplication::setOrganizationName(BuildInfo::MODIFIED_ORGANIZATION); + QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); + QCoreApplication::setApplicationVersion(BuildInfo::VERSION); + + setupGlobalInstances(); + +#ifndef WIN32 + setvbuf(stdout, NULL, _IOLBF, 0); +#endif + + qInstallMessageHandler(LogHandler::verboseMessageHandler); + qInfo() << "Starting."; +} #ifdef Q_OS_WIN QString getLastErrorAsString() { diff --git a/libraries/shared/src/SharedUtil.h b/libraries/shared/src/SharedUtil.h index 64bbc6585b..7cb750d3e3 100644 --- a/libraries/shared/src/SharedUtil.h +++ b/libraries/shared/src/SharedUtil.h @@ -228,6 +228,7 @@ void watchParentProcess(int parentPID); bool processIsRunning(int64_t pid); +void setupHifiApplication(QString applicationName); #ifdef Q_OS_WIN void* createProcessGroup(); diff --git a/tests/entities/src/main.cpp b/tests/entities/src/main.cpp index 792ef7d9c6..bf79f9d3e9 100644 --- a/tests/entities/src/main.cpp +++ b/tests/entities/src/main.cpp @@ -20,6 +20,7 @@ #include #include #include +#include const QString& getTestResourceDir() { static QString dir; @@ -136,6 +137,8 @@ void testPropertyFlags() { } int main(int argc, char** argv) { + setupHifiApplication("Entities Test"); + QCoreApplication app(argc, argv); { auto start = usecTimestampNow(); diff --git a/tests/gpu-test/src/main.cpp b/tests/gpu-test/src/main.cpp index 6a509afe4e..77ce015e3f 100644 --- a/tests/gpu-test/src/main.cpp +++ b/tests/gpu-test/src/main.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include @@ -192,7 +193,9 @@ void testSparseRectify() { } } -int main(int argc, char** argv) { +int main(int argc, char** argv) { + setupHifiApplication("GPU Test"); + testSparseRectify(); // FIXME this test appears to be broken diff --git a/tests/ktx/src/KtxTests.cpp b/tests/ktx/src/KtxTests.cpp index 94e5d7e8e7..65d9cbec3d 100644 --- a/tests/ktx/src/KtxTests.cpp +++ b/tests/ktx/src/KtxTests.cpp @@ -149,7 +149,8 @@ static const QString TEST_FOLDER { "H:/ktx_cacheold" }; static const QString EXTENSIONS { "*.ktx" }; int mainTemp(int, char**) { - qInstallMessageHandler(messageHandler); + setupHifiApplication("KTX Tests"); + auto fileInfoList = QDir { TEST_FOLDER }.entryInfoList(QStringList { EXTENSIONS }); for (auto fileInfo : fileInfoList) { qDebug() << fileInfo.filePath(); diff --git a/tests/qml/src/main.cpp b/tests/qml/src/main.cpp index 219efa5996..022f7290f4 100644 --- a/tests/qml/src/main.cpp +++ b/tests/qml/src/main.cpp @@ -179,18 +179,10 @@ void TestWindow::resizeEvent(QResizeEvent* ev) { resizeWindow(ev->size()); } -void messageHandler(QtMsgType type, const QMessageLogContext& context, const QString& message) { - if (!message.isEmpty()) { -#ifdef Q_OS_WIN - OutputDebugStringA(message.toLocal8Bit().constData()); - OutputDebugStringA("\n"); -#endif - } -} +int main(int argc, char** argv) { + setupHifiApplication("QML Test"); -int main(int argc, char** argv) { QGuiApplication app(argc, argv); - qInstallMessageHandler(messageHandler); TestWindow window; app.exec(); return 0; diff --git a/tests/qt59/src/main.cpp b/tests/qt59/src/main.cpp index 7b95cabd6c..19b922de9f 100644 --- a/tests/qt59/src/main.cpp +++ b/tests/qt59/src/main.cpp @@ -63,14 +63,11 @@ void Qt59TestApp::finish(int exitCode) { int main(int argc, char * argv[]) { - QCoreApplication::setApplicationName("Qt59Test"); - QCoreApplication::setOrganizationName(BuildInfo::MODIFIED_ORGANIZATION); - QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); - QCoreApplication::setApplicationVersion(BuildInfo::VERSION); + setupHifiApplication("Qt59Test"); Qt59TestApp app(argc, argv); return app.exec(); } -#include "main.moc" \ No newline at end of file +#include "main.moc" diff --git a/tests/recording/src/main.cpp b/tests/recording/src/main.cpp index f4049b04b7..1b4e5adf6d 100644 --- a/tests/recording/src/main.cpp +++ b/tests/recording/src/main.cpp @@ -10,6 +10,8 @@ #include #include +#include + #include "Constants.h" using namespace recording; @@ -97,18 +99,10 @@ void testClipOrdering() { Q_UNUSED(lastFrameTimeOffset); // FIXME - Unix build not yet upgraded to Qt 5.5.1 we can remove this once it is } -#ifdef Q_OS_WIN32 -void myMessageHandler(QtMsgType type, const QMessageLogContext & context, const QString & msg) { - OutputDebugStringA(msg.toLocal8Bit().toStdString().c_str()); - OutputDebugStringA("\n"); -} -#endif - int main(int, const char**) { -#ifdef Q_OS_WIN32 - qInstallMessageHandler(myMessageHandler); -#endif + setupHifiApplication("Recording Test"); + testFrameTypeRegistration(); testFilePersist(); testClipOrdering(); -} \ No newline at end of file +} diff --git a/tests/render-perf/src/main.cpp b/tests/render-perf/src/main.cpp index 2bf0c74067..ec56555f68 100644 --- a/tests/render-perf/src/main.cpp +++ b/tests/render-perf/src/main.cpp @@ -1138,31 +1138,17 @@ private: bool QTestWindow::_cullingEnabled = true; -void messageHandler(QtMsgType type, const QMessageLogContext& context, const QString& message) { - QString logMessage = LogHandler::getInstance().printMessage((LogMsgType)type, context, message); - - if (!logMessage.isEmpty()) { -#ifdef Q_OS_WIN - OutputDebugStringA(logMessage.toLocal8Bit().constData()); - OutputDebugStringA("\n"); -#endif - logger->addMessage(qPrintable(logMessage + "\n")); - } -} - const char * LOG_FILTER_RULES = R"V0G0N( hifi.gpu=true )V0G0N"; int main(int argc, char** argv) { + setupHifiApplication("RenderPerf"); + QApplication app(argc, argv); - QCoreApplication::setApplicationName("RenderPerf"); - QCoreApplication::setOrganizationName("High Fidelity"); - QCoreApplication::setOrganizationDomain("highfidelity.com"); logger.reset(new FileLogger()); - qInstallMessageHandler(messageHandler); QLoggingCategory::setFilterRules(LOG_FILTER_RULES); QTestWindow::setup(); QTestWindow window; diff --git a/tests/render-texture-load/src/main.cpp b/tests/render-texture-load/src/main.cpp index 066b39fc40..62c970cab5 100644 --- a/tests/render-texture-load/src/main.cpp +++ b/tests/render-texture-load/src/main.cpp @@ -610,16 +610,6 @@ private: bool _ready { false }; }; -void messageHandler(QtMsgType type, const QMessageLogContext& context, const QString& message) { - if (!message.isEmpty()) { -#ifdef Q_OS_WIN - OutputDebugStringA(message.toLocal8Bit().constData()); - OutputDebugStringA("\n"); -#endif - std::cout << message.toLocal8Bit().constData() << std::endl; - } -} - const char * LOG_FILTER_RULES = R"V0G0N( hifi.gpu=true )V0G0N"; @@ -645,11 +635,9 @@ void unzipTestData(const QByteArray& zipData) { } int main(int argc, char** argv) { + setupHifiApplication("RenderPerf"); + QApplication app(argc, argv); - QCoreApplication::setApplicationName("RenderPerf"); - QCoreApplication::setOrganizationName("High Fidelity"); - QCoreApplication::setOrganizationDomain("highfidelity.com"); - qInstallMessageHandler(messageHandler); QLoggingCategory::setFilterRules(LOG_FILTER_RULES); if (!DATA_DIR.exists()) { diff --git a/tests/render-utils/src/main.cpp b/tests/render-utils/src/main.cpp index 741fdbdddd..e30a80f3d9 100644 --- a/tests/render-utils/src/main.cpp +++ b/tests/render-utils/src/main.cpp @@ -179,25 +179,14 @@ void QTestWindow::draw() { } } -void messageHandler(QtMsgType type, const QMessageLogContext& context, const QString& message) { - if (!message.isEmpty()) { -#ifdef Q_OS_WIN - OutputDebugStringA(message.toLocal8Bit().constData()); - OutputDebugStringA("\n"); -#else - std::cout << message.toLocal8Bit().constData() << std::endl; -#endif - } -} - - const char * LOG_FILTER_RULES = R"V0G0N( hifi.gpu=true )V0G0N"; -int main(int argc, char** argv) { +int main(int argc, char** argv) { + setupHifiApplication("Render Utils Test"); + QGuiApplication app(argc, argv); - qInstallMessageHandler(messageHandler); QLoggingCategory::setFilterRules(LOG_FILTER_RULES); QTestWindow window; QTimer timer; diff --git a/tests/shaders/src/main.cpp b/tests/shaders/src/main.cpp index 585bc432a9..a3b4196031 100644 --- a/tests/shaders/src/main.cpp +++ b/tests/shaders/src/main.cpp @@ -214,24 +214,14 @@ void QTestWindow::draw() { _context.swapBuffers(this); } -void messageHandler(QtMsgType type, const QMessageLogContext& context, const QString& message) { - if (!message.isEmpty()) { -#ifdef Q_OS_WIN - OutputDebugStringA(message.toLocal8Bit().constData()); - OutputDebugStringA("\n"); -#else - std::cout << message.toLocal8Bit().constData() << std::endl; -#endif - } -} - const char * LOG_FILTER_RULES = R"V0G0N( hifi.gpu=true )V0G0N"; -int main(int argc, char** argv) { +int main(int argc, char** argv) { + setupHifiApplication("Shaders Test"); + QGuiApplication app(argc, argv); - qInstallMessageHandler(messageHandler); QLoggingCategory::setFilterRules(LOG_FILTER_RULES); QTestWindow window; QTimer timer; diff --git a/tools/ac-client/src/main.cpp b/tools/ac-client/src/main.cpp index 139e44bc9a..024a891d3c 100644 --- a/tools/ac-client/src/main.cpp +++ b/tools/ac-client/src/main.cpp @@ -20,12 +20,7 @@ using namespace std; int main(int argc, char* argv[]) { - QCoreApplication::setApplicationName(BuildInfo::AC_CLIENT_SERVER_NAME); - QCoreApplication::setOrganizationName(BuildInfo::MODIFIED_ORGANIZATION); - QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); - QCoreApplication::setApplicationVersion(BuildInfo::VERSION); - - setupGlobalInstances(); + setupHifiApplication(BuildInfo::AC_CLIENT_SERVER_NAME); Setting::init(); diff --git a/tools/atp-client/src/main.cpp b/tools/atp-client/src/main.cpp index 0a8274fedd..2a267c088c 100644 --- a/tools/atp-client/src/main.cpp +++ b/tools/atp-client/src/main.cpp @@ -20,12 +20,7 @@ using namespace std; int main(int argc, char * argv[]) { - QCoreApplication::setApplicationName(BuildInfo::AC_CLIENT_SERVER_NAME); - QCoreApplication::setOrganizationName(BuildInfo::MODIFIED_ORGANIZATION); - QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); - QCoreApplication::setApplicationVersion(BuildInfo::VERSION); - - setupGlobalInstances(); + setupHifiApplication("ATP Client"); Setting::init(); diff --git a/tools/auto-tester/src/main.cpp b/tools/auto-tester/src/main.cpp index 45a3743482..6e5e06b732 100644 --- a/tools/auto-tester/src/main.cpp +++ b/tools/auto-tester/src/main.cpp @@ -17,4 +17,4 @@ int main(int argc, char *argv[]) { autoTester.show(); return application.exec(); -} \ No newline at end of file +} diff --git a/tools/ice-client/src/main.cpp b/tools/ice-client/src/main.cpp index c70a7eb7d7..60a5d4e0e4 100644 --- a/tools/ice-client/src/main.cpp +++ b/tools/ice-client/src/main.cpp @@ -13,11 +13,15 @@ #include #include +#include + #include "ICEClientApp.h" using namespace std; int main(int argc, char * argv[]) { + setupHifiApplication("ICE Client"); + ICEClientApp app(argc, argv); return app.exec(); } diff --git a/tools/oven/src/main.cpp b/tools/oven/src/main.cpp index be0c22a0c6..3f4afe1f15 100644 --- a/tools/oven/src/main.cpp +++ b/tools/oven/src/main.cpp @@ -15,12 +15,7 @@ #include int main (int argc, char** argv) { - QCoreApplication::setApplicationName("Oven"); - QCoreApplication::setOrganizationName(BuildInfo::MODIFIED_ORGANIZATION); - QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); - QCoreApplication::setApplicationVersion(BuildInfo::VERSION); - - setupGlobalInstances(); + setupHifiApplication("Oven"); // init the settings interface so we can save and load settings Setting::init(); diff --git a/tools/skeleton-dump/src/main.cpp b/tools/skeleton-dump/src/main.cpp index 6cf4d41f31..d5919c4a88 100644 --- a/tools/skeleton-dump/src/main.cpp +++ b/tools/skeleton-dump/src/main.cpp @@ -14,9 +14,13 @@ #include #include +#include + #include "SkeletonDumpApp.h" int main(int argc, char * argv[]) { + setupHifiApplication("Skeleton Dump App"); + SkeletonDumpApp app(argc, argv); return app.getReturnCode(); } diff --git a/tools/udt-test/src/UDTTest.cpp b/tools/udt-test/src/UDTTest.cpp index 6161dbfdbc..ce89f04ce5 100644 --- a/tools/udt-test/src/UDTTest.cpp +++ b/tools/udt-test/src/UDTTest.cpp @@ -71,8 +71,6 @@ const QStringList SERVER_STATS_TABLE_HEADERS { UDTTest::UDTTest(int& argc, char** argv) : QCoreApplication(argc, argv) { - qInstallMessageHandler(LogHandler::verboseMessageHandler); - parseArguments(); // randomize the seed for packet size randomization diff --git a/tools/udt-test/src/main.cpp b/tools/udt-test/src/main.cpp index ccb7d0af0f..d88218c0d0 100644 --- a/tools/udt-test/src/main.cpp +++ b/tools/udt-test/src/main.cpp @@ -10,9 +10,13 @@ #include +#include + #include "UDTTest.h" int main(int argc, char* argv[]) { + setupHifiApplication("UDT Test); + UDTTest app(argc, argv); return app.exec(); } diff --git a/tools/vhacd-util/src/main.cpp b/tools/vhacd-util/src/main.cpp index 42c9db9513..817e77bf8e 100644 --- a/tools/vhacd-util/src/main.cpp +++ b/tools/vhacd-util/src/main.cpp @@ -15,6 +15,8 @@ #include #include +#include + #include "VHACDUtilApp.h" using namespace std; @@ -22,6 +24,8 @@ using namespace VHACD; int main(int argc, char * argv[]) { + setupHifiApplication("VHACD Util"); + VHACDUtilApp app(argc, argv); return app.getReturnCode(); } From 800d15b4055d30076855735652af5f082cdd40db Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Fri, 23 Feb 2018 16:23:50 -0800 Subject: [PATCH 3/4] Make sure settings timer is running --- interface/src/Application.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index e956195ca6..3dd75e2c25 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1512,6 +1512,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo settingsTimer->setSingleShot(false); settingsTimer->setInterval(SAVE_SETTINGS_INTERVAL); // 10s, Qt::CoarseTimer acceptable QObject::connect(settingsTimer, &QTimer::timeout, this, &Application::saveSettings); + settingsTimer->start(); }, QThread::LowestPriority); if (Menu::getInstance()->isOptionChecked(MenuOption::FirstPerson)) { From 2a204533f4344ccf4f116de169f6fd2ff75087ea Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Mon, 26 Feb 2018 15:56:11 -0800 Subject: [PATCH 4/4] Add comments to startup processes --- libraries/shared/src/LogHandler.cpp | 1 + libraries/shared/src/SettingInterface.cpp | 31 ++++++++++++--------- libraries/shared/src/SettingManager.h | 4 +-- libraries/shared/src/SharedUtil.cpp | 33 ++++++++++++++++++++--- libraries/shared/src/SharedUtil.h | 5 ++++ 5 files changed, 56 insertions(+), 18 deletions(-) diff --git a/libraries/shared/src/LogHandler.cpp b/libraries/shared/src/LogHandler.cpp index ff0d68dcce..cb3c0d07b2 100644 --- a/libraries/shared/src/LogHandler.cpp +++ b/libraries/shared/src/LogHandler.cpp @@ -192,6 +192,7 @@ QString LogHandler::printMessage(LogMsgType type, const QMessageLogContext& cont fprintf(stdout, "%s", qPrintable(logMessage)); #ifdef Q_OS_WIN + // On windows, this will output log lines into the Visual Studio "output" tab OutputDebugStringA(qPrintable(logMessage)); #endif return logMessage; diff --git a/libraries/shared/src/SettingInterface.cpp b/libraries/shared/src/SettingInterface.cpp index 62f116795e..04da35656e 100644 --- a/libraries/shared/src/SettingInterface.cpp +++ b/libraries/shared/src/SettingInterface.cpp @@ -23,25 +23,27 @@ #include "SharedUtil.h" namespace Setting { - // cleans up the settings private instance. Should only be run once at closing down. - void cleanupPrivateInstance() { + // This should only run as a post-routine in the QCoreApplication destructor + void cleanupSettingsSaveThread() { auto globalManager = DependencyManager::get(); Q_ASSERT(qApp && globalManager); - // grab the thread before we nuke the instance + // Grab the settings thread to shut it down QThread* settingsManagerThread = globalManager->thread(); - // quit the settings manager thread + // Quit the settings manager thread and wait for it so we + // don't get concurrent accesses when we save all settings below settingsManagerThread->quit(); settingsManagerThread->wait(); - // Save all settings + // [IMPORTANT] Save all settings when the QApplication goes down globalManager->saveAll(); qCDebug(shared) << "Settings thread stopped."; } - void setupPrivateInstance() { + // This should only run as a pre-routine in the QCoreApplication constructor + void setupSettingsSaveThread() { auto globalManager = DependencyManager::get(); Q_ASSERT(qApp && globalManager); @@ -55,6 +57,9 @@ namespace Setting { QObject::connect(thread, &QThread::finished, globalManager.data(), &Manager::stopTimer); // Setup manager threading affinity + // This makes the timer fire on the settings thread so we don't block the main + // thread with a lot of file I/O. + // We bring back the manager to the main thread when the QApplication goes down globalManager->moveToThread(thread); QObject::connect(thread, &QThread::finished, globalManager.data(), [] { auto globalManager = DependencyManager::get(); @@ -63,15 +68,17 @@ namespace Setting { // Move manager back to the main thread (has to be done on owning thread) globalManager->moveToThread(qApp->thread()); }); - + + // Start the settings save thread thread->start(); qCDebug(shared) << "Settings thread started."; - // Register cleanupPrivateInstance to run inside QCoreApplication's destructor. - qAddPostRoutine(cleanupPrivateInstance); + // Register cleanupSettingsSaveThread to run inside QCoreApplication's destructor. + // This will cleanup the settings thread and save all settings before shut down. + qAddPostRoutine(cleanupSettingsSaveThread); } - // Sets up the settings private instance. Should only be run once at startup. preInit() must be run beforehand, + // Sets up the settings private instance. Should only be run once at startup. void init() { // Set settings format QSettings::setDefaultFormat(JSON_FORMAT); @@ -91,11 +98,11 @@ namespace Setting { qCDebug(shared) << (deleted ? "Deleted" : "Failed to delete") << "settings lock file" << settingsLockFilename; } - // Setup settings manager + // Setup settings manager, the manager will live until the process shuts down DependencyManager::set(); // Add pre-routine to setup threading - qAddPreRoutine(setupPrivateInstance); + qAddPreRoutine(setupSettingsSaveThread); } void Interface::init() { diff --git a/libraries/shared/src/SettingManager.h b/libraries/shared/src/SettingManager.h index ffdd4ba42a..6696a1ecf4 100644 --- a/libraries/shared/src/SettingManager.h +++ b/libraries/shared/src/SettingManager.h @@ -50,8 +50,8 @@ namespace Setting { QHash _pendingChanges; friend class Interface; - friend void cleanupPrivateInstance(); - friend void setupPrivateInstance(); + friend void cleanupSettingsSaveThread(); + friend void setupSettingsSaveThread(); }; } diff --git a/libraries/shared/src/SharedUtil.cpp b/libraries/shared/src/SharedUtil.cpp index d05e940322..412ea59dfe 100644 --- a/libraries/shared/src/SharedUtil.cpp +++ b/libraries/shared/src/SharedUtil.cpp @@ -66,10 +66,23 @@ extern "C" FILE * __cdecl __iob_func(void) { #include "OctalCode.h" #include "SharedLogging.h" +// Global instances are stored inside the QApplication properties +// to provide a single instance across DLL boundaries. +// This is something we cannot do here since several DLLs +// and our main binaries statically link this "shared" library +// resulting in multiple static memory blocks in different constexts +// But we need to be able to use global instances before the QApplication +// is setup, so to accomplish that we stage the global instances in a local +// map and setup a pre routine (commitGlobalInstances) that will run in the +// QApplication constructor and commit all the staged instances to the +// QApplication properties. +// Note: One of the side effects of this, is that no DLL loaded before +// the QApplication is constructed, can expect to access the existing staged +// global instanced. For this reason, we advise all DLLs be loaded after +// the QApplication is instanced. static std::mutex stagedGlobalInstancesMutex; static std::unordered_map stagedGlobalInstances; - std::mutex& globalInstancesMutex() { return stagedGlobalInstancesMutex; } @@ -82,6 +95,9 @@ static void commitGlobalInstances() { stagedGlobalInstances.clear(); } +// This call is necessary for global instances to work across DLL boundaries +// Ideally, this founction would be called at the top of the main function. +// See description at the top of the file. void setupGlobalInstances() { qAddPreRoutine(commitGlobalInstances); } @@ -819,8 +835,8 @@ bool similarStrings(const QString& stringA, const QString& stringB) { } void disableQtBearerPoll() { - // to disable the Qt constant wireless scanning, set the env for polling interval - qDebug() << "Disabling Qt wireless polling by using a negative value for QTimer::setInterval"; + // To disable the Qt constant wireless scanning, set the env for polling interval to -1 + // The constant polling causes ping spikes on windows every 10 seconds or so that affect the audio const QByteArray DISABLE_BEARER_POLL_TIMEOUT = QString::number(-1).toLocal8Bit(); qputenv("QT_BEARER_POLL_TIMEOUT", DISABLE_BEARER_POLL_TIMEOUT); } @@ -1185,19 +1201,28 @@ void watchParentProcess(int parentPID) { void setupHifiApplication(QString applicationName) { disableQtBearerPoll(); // Fixes wifi ping spikes + // Those calls are necessary to format the log correctly + // and to direct the application to the correct location + // for read/writes into AppData and other platform equivalents. QCoreApplication::setApplicationName(applicationName); QCoreApplication::setOrganizationName(BuildInfo::MODIFIED_ORGANIZATION); QCoreApplication::setOrganizationDomain(BuildInfo::ORGANIZATION_DOMAIN); QCoreApplication::setApplicationVersion(BuildInfo::VERSION); + // This ensures the global instances mechanism is correctly setup. + // You can find more details as to why this is important in the SharedUtil.h/cpp files setupGlobalInstances(); #ifndef WIN32 + // Windows tends to hold onto log lines until it has a sizeable buffer + // This makes the log feel unresponsive and trap useful log data in the log buffer + // when a crash occurs. + //Force windows to flush the buffer on each new line character to avoid this. setvbuf(stdout, NULL, _IOLBF, 0); #endif + // Install the standard hifi message handler so we get consistant log formatting qInstallMessageHandler(LogHandler::verboseMessageHandler); - qInfo() << "Starting."; } #ifdef Q_OS_WIN diff --git a/libraries/shared/src/SharedUtil.h b/libraries/shared/src/SharedUtil.h index 7cb750d3e3..51a84bed06 100644 --- a/libraries/shared/src/SharedUtil.h +++ b/libraries/shared/src/SharedUtil.h @@ -36,7 +36,12 @@ std::unique_ptr& globalInstancePointer() { return instancePtr; } +// Sets up the global instances for use +// This NEEDS to be called on startup +// for any binary planing on using global instances +// More details in cpp file void setupGlobalInstances(); + std::mutex& globalInstancesMutex(); QVariant getGlobalInstance(const char* propertyName); void setGlobalInstance(const char* propertyName, const QVariant& variant);