Add comments to startup processes

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;

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<Manager>();
         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<Manager>();
         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<Manager>();
@@ -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<Manager>();
 
         // Add pre-routine to setup threading
-        qAddPreRoutine(setupPrivateInstance);
+        qAddPreRoutine(setupSettingsSaveThread);
     }
 
     void Interface::init() {

libraries/shared/src/SettingManager.h

@@ -50,8 +50,8 @@ namespace Setting {
         QHash<QString, QVariant> _pendingChanges;
 
         friend class Interface;
-        friend void cleanupPrivateInstance();
-        friend void setupPrivateInstance();
+        friend void cleanupSettingsSaveThread();
+        friend void setupSettingsSaveThread();
     };
 }
 

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<std::string, QVariant> 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

libraries/shared/src/SharedUtil.h

@@ -36,7 +36,12 @@ std::unique_ptr<T>& 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);