Improve the update checker so it can be opt-out (#696)

* Add a cvar to enable/disable update checking

* Refactor update checker

This splits the thread part from the main part to clarify the code

* Move and update the configuration documentation, for a more general approach

code/qcommon/common.c

@@ -127,7 +127,8 @@ cvar_t	*precache;
 cvar_t	*com_target_game;
 cvar_t	*com_target_version;
 cvar_t	*com_target_demo;
-cvar_t	*com_updateCheckInterval;
+cvar_t	*com_updatecheck_enabled;
+cvar_t	*com_updatecheck_interval;
 
 int protocol_version_demo;
 int protocol_version_full;
@@ -1919,8 +1920,9 @@ void Com_Init( char *commandLine ) {
 #ifdef LEGACY_PROTOCOL
 	com_legacyprotocol = Cvar_Get("com_legacyprotocol", va("%i", PROTOCOL_LEGACY_VERSION), CVAR_INIT);
 
-	com_updateCheckInterval = Cvar_Get("com_updateCheckInterval", "15", 0);
-    Cvar_CheckRange(com_updateCheckInterval, 5, 240, qtrue);
+	com_updatecheck_enabled = Cvar_Get("com_updatecheck_enabled", "1", CVAR_ARCHIVE);
+	com_updatecheck_interval = Cvar_Get("com_updatecheck_interval", "15", 0);
+    Cvar_CheckRange(com_updatecheck_interval, 5, 240, qtrue);
 
 	// Keep for compatibility with old mods / mods that haven't updated yet.
 	if(com_legacyprotocol->integer > 0)

code/qcommon/qcommon.h

@@ -1060,7 +1060,8 @@ extern  cvar_t* con_autochat;
 extern	cvar_t* com_target_version;
 extern	cvar_t* com_target_game;
 extern	cvar_t* com_target_demo;
-extern	cvar_t* com_updateCheckInterval;
+extern	cvar_t* com_updatecheck_enabled;
+extern	cvar_t* com_updatecheck_interval;
 
 extern	int		protocol_version_demo;
 extern	int		protocol_version_full;

code/sys/sys_update_checker.cpp

@@ -54,53 +54,29 @@ UpdateChecker::UpdateChecker()
 {
     lastMajor = lastMinor = lastPatch = 0;
     versionChecked                    = false;
-    handle                            = NULL;
-    thread                            = NULL;
-    requestThreadIsActive             = qfalse;
 }
 
-UpdateChecker::~UpdateChecker()
-{
-    if (handle) {
-        Shutdown();
-    }
-}
+UpdateChecker::~UpdateChecker() {}
 
 void UpdateChecker::Init()
 {
-#ifdef HAS_LIBCURL
-    CURLcode result;
-
-    assert(!handle);
-    assert(!thread);
-
-    handle = curl_easy_init();
-    if (!handle) {
-        Com_DPrintf("Failed to create curl client\n");
-        return;
-    }
-
-    result = curl_easy_setopt(handle, CURLOPT_URL, "https://api.github.com/repos/openmoh/openmohaa/releases/latest");
-
-    if (result != CURLE_OK) {
-        Com_DPrintf("Failed to set curl URL: %s\n", curl_easy_strerror(result));
-        curl_easy_cleanup(handle);
-        handle = NULL;
-        return;
-    }
-
-    curl_easy_setopt(handle, CURLOPT_USERAGENT, "curl");
-#endif
-
     CheckInitClientThread();
 }
 
 void UpdateChecker::CheckInitClientThread()
 {
-    if (!requestThreadIsActive && CanHaveRequestThread()) {
-        requestThreadIsActive = qtrue;
+    if (!thread) {
+        if (CanHaveRequestThread()) {
+            thread = new UpdateCheckerThread();
+            thread->Init();
+        }
+    } else {
+        if (!CanHaveRequestThread()) {
+            Com_DPrintf("Shutting down the update checker thread\n");
 
-        thread = new std::thread(&UpdateChecker::RequestThread, this);
+            delete thread;
+            thread = NULL;
+        }
     }
 }
 
@@ -111,6 +87,11 @@ bool UpdateChecker::CanHaveRequestThread() const
         return false;
     }
 
+    if (!com_updatecheck_enabled->integer) {
+        // Update checking has been disabled
+        return false;
+    }
+
 #ifdef HAS_LIBCURL
     return true;
 #else
@@ -118,21 +99,28 @@ bool UpdateChecker::CanHaveRequestThread() const
 #endif
 }
 
+void UpdateChecker::SetLatestVersion(int major, int minor, int patch)
+{
+    lastMajor = major;
+    lastMinor = minor;
+    lastPatch = patch;
+
+    versionChecked = true;
+}
+
 void UpdateChecker::Process()
 {
     // Initialize the client thread when necessary
     CheckInitClientThread();
 
     std::chrono::time_point<std::chrono::steady_clock> currentTime = std::chrono::steady_clock::now();
-    if (currentTime
-        < lastMessageTime + std::chrono::milliseconds(Q_max(1, com_updateCheckInterval->integer) * 60 * 1000)) {
+    if (currentTime < nextMessageTime) {
         return;
     }
 
     if (!CheckNewVersion()) {
         return;
     }
-    lastMessageTime = currentTime;
 
     Com_Printf(
         "New release v%d.%d.%d published *\\(^ o ^)/*. Your current version is v%s. See www.openmohaa.org\n",
@@ -141,37 +129,16 @@ void UpdateChecker::Process()
         lastPatch,
         PRODUCT_VERSION_NUMBER_STRING
     );
+
+    nextMessageTime = currentTime + std::chrono::milliseconds(Q_max(1, com_updatecheck_interval->integer) * 60 * 1000);
 }
 
 void UpdateChecker::Shutdown()
 {
-    ShutdownClient();
-    ShutdownThread();
-}
-
-void UpdateChecker::ShutdownClient()
-{
-#ifdef HAS_LIBCURL
-    std::lock_guard<std::shared_mutex> l(clientMutex);
-
-    if (!handle) {
-        return;
+    if (thread) {
+        delete thread;
+        thread = NULL;
     }
-
-    curl_easy_cleanup(handle);
-    handle = NULL;
-#endif
-}
-
-void UpdateChecker::ShutdownThread()
-{
-    if (!thread) {
-        return;
-    }
-
-    thread->join();
-    delete thread;
-    thread = NULL;
 }
 
 bool UpdateChecker::CheckNewVersion() const
@@ -212,7 +179,91 @@ bool UpdateChecker::CheckNewVersion(int& major, int& minor, int& patch) const
     return true;
 }
 
-bool UpdateChecker::ParseVersionNumber(const char *value, int& major, int& minor, int& patch) const
+UpdateCheckerThread::UpdateCheckerThread()
+{
+    handle                = NULL;
+    osThread              = NULL;
+    requestThreadIsActive = qfalse;
+    shouldBeActive        = qfalse;
+}
+
+UpdateCheckerThread::~UpdateCheckerThread()
+{
+    Shutdown();
+}
+
+void UpdateCheckerThread::Init()
+{
+    shouldBeActive        = qtrue;
+    requestThreadIsActive = qtrue;
+
+    osThread = new std::thread(&UpdateCheckerThread::RequestThread, this);
+}
+
+void UpdateCheckerThread::Shutdown()
+{
+    if (!shouldBeActive) {
+        return;
+    }
+
+    shouldBeActive = qfalse;
+
+    if (osThread) {
+        // Notify and shutdown the thread
+        clientWake.notify_all();
+        osThread->join();
+
+        delete osThread;
+        osThread = NULL;
+    }
+}
+
+bool UpdateCheckerThread::IsRoutineActive() const
+{
+    return requestThreadIsActive;
+}
+
+void UpdateCheckerThread::InitClient()
+{
+#ifdef HAS_LIBCURL
+    CURLcode result;
+
+    assert(!handle);
+
+    handle = curl_easy_init();
+    if (!handle) {
+        Com_DPrintf("Failed to create curl client\n");
+        return;
+    }
+
+    result = curl_easy_setopt(handle, CURLOPT_URL, "https://api.github.com/repos/openmoh/openmohaa/releases/latest");
+
+    if (result != CURLE_OK) {
+        Com_DPrintf("Failed to set curl URL: %s\n", curl_easy_strerror(result));
+        curl_easy_cleanup(handle);
+        handle = NULL;
+        return;
+    }
+
+    curl_easy_setopt(handle, CURLOPT_USERAGENT, "curl");
+#else
+    Com_DPrintf("Project was compiled without libcurl, will not check for updates\n");
+#endif
+}
+
+void UpdateCheckerThread::ShutdownClient()
+{
+#ifdef HAS_LIBCURL
+    if (!handle) {
+        return;
+    }
+
+    curl_easy_cleanup(handle);
+    handle = NULL;
+#endif
+}
+
+bool UpdateCheckerThread::ParseVersionNumber(const char *value, int& major, int& minor, int& patch) const
 {
     const char *p  = value;
     const char *pn = value;
@@ -268,12 +319,11 @@ size_t WriteCallback(char *contents, size_t size, size_t nmemb, void *userp)
     return size * nmemb;
 }
 
-void UpdateChecker::DoRequest()
+void UpdateCheckerThread::DoRequest()
 {
 #ifdef HAS_LIBCURL
-    std::lock_guard<std::shared_mutex> l(clientMutex);
-    CURLcode                           result;
-    std::string                        responseString;
+    CURLcode    result;
+    std::string responseString;
 
     curl_easy_setopt(handle, CURLOPT_WRITEFUNCTION, &WriteCallback);
     curl_easy_setopt(handle, CURLOPT_WRITEDATA, &responseString);
@@ -297,31 +347,30 @@ void UpdateChecker::DoRequest()
         int major, minor, patch;
         ParseVersionNumber(tagName.c_str(), major, minor, patch);
 
-        lastMajor = major;
-        lastMinor = minor;
-        lastPatch = patch;
-
-        versionChecked = true;
+        updateChecker.SetLatestVersion(major, minor, patch);
     } catch (std::out_of_range&) {}
 #endif
 }
 
-void UpdateChecker::RequestThread()
+void UpdateCheckerThread::RequestThread()
 {
-    std::chrono::time_point<std::chrono::steady_clock> currentTime = std::chrono::steady_clock::now();
-    std::chrono::time_point<std::chrono::steady_clock> lastCheckTime;
+    // Initialize the curl client
+    InitClient();
 
-    while (handle && CanHaveRequestThread()) {
-        currentTime = std::chrono::steady_clock::now();
-        if (currentTime
-            >= lastCheckTime + std::chrono::milliseconds(Q_max(1, com_updateCheckInterval->integer) * 60 * 1000)) {
-            lastCheckTime = currentTime;
-
-            DoRequest();
-        }
-
-        std::this_thread::sleep_for(std::chrono::seconds(5));
+    while (handle && shouldBeActive) {
+        DoRequest();
+        RequestThreadSleep();
     }
 
+    ShutdownClient();
+
     requestThreadIsActive = qfalse;
 }
+
+void UpdateCheckerThread::RequestThreadSleep()
+{
+    const std::chrono::seconds interval = std::chrono::seconds(Q_max(1, com_updatecheck_interval->integer) * 60);
+
+    std::unique_lock<std::mutex> l(clientWakeMutex);
+    clientWake.wait_for(l, interval);
+}

code/sys/sys_update_checker.h

@@ -41,6 +41,37 @@ extern "C" {
 #    include <thread>
 #    include <shared_mutex>
 #    include <chrono>
+#    include <condition_variable>
+
+class UpdateCheckerThread
+{
+public:
+    UpdateCheckerThread();
+    ~UpdateCheckerThread();
+
+    void Init();
+    void Shutdown();
+    bool IsRoutineActive() const;
+
+private:
+    void ShutdownThread();
+
+    void InitClient();
+    void ShutdownClient();
+
+    bool ParseVersionNumber(const char *value, int& major, int& minor, int& patch) const;
+    void RequestThread();
+    void RequestThreadSleep();
+    void DoRequest();
+
+private:
+    void                   *handle;
+    std::mutex              clientWakeMutex;
+    std::condition_variable clientWake;
+    std::thread            *osThread;
+    qboolean                requestThreadIsActive;
+    qboolean                shouldBeActive;
+};
 
 class UpdateChecker
 {
@@ -54,15 +85,11 @@ public:
 
     bool CheckNewVersion() const;
     bool CheckNewVersion(int& major, int& minor, int& patch) const;
+    void SetLatestVersion(int major, int minor, int patch);
 
 private:
-    void ShutdownClient();
-    void ShutdownThread();
-    void RequestThread();
     void CheckInitClientThread();
     bool CanHaveRequestThread() const;
-    void DoRequest();
-    bool ParseVersionNumber(const char *value, int& major, int& minor, int& patch) const;
 
 private:
     //
@@ -73,15 +100,9 @@ private:
     int  lastPatch;
     bool versionChecked;
 
-    std::chrono::time_point<std::chrono::steady_clock> lastMessageTime;
+    std::chrono::time_point<std::chrono::steady_clock> nextMessageTime;
 
-    //
-    // Thread-related variables
-    //
-    void             *handle;
-    std::shared_mutex clientMutex;
-    std::thread      *thread;
-    qboolean          requestThreadIsActive;
+    UpdateCheckerThread *thread;
 };
 
 extern UpdateChecker updateChecker;