* common helper for killable threads + remove dedicated class for background thread

* avoid overriding SteamPath env var in `SteamAPI_GetSteamInstallPath()`

dll/dll.cpp

@@ -161,6 +161,7 @@ STEAMAPI_API HSteamUser SteamAPI_GetHSteamUser()
     return CLIENT_HSTEAMUSER;
 }
 
+
 // declare "g_pSteamClientGameServer" as an export for API library, then actually define it
 #if !defined(STEAMCLIENT_DLL) // api
 STEAMAPI_API ISteamClient *g_pSteamClientGameServer;
@@ -600,10 +601,16 @@ STEAMAPI_API HSteamUser Steam_GetHSteamUserCurrent()
 STEAMAPI_API const char *SteamAPI_GetSteamInstallPath()
 {
     PRINT_DEBUG_ENTRY();
-    static char steam_folder[1024];
+    static char steam_folder[4096]{};
-    std::string path = Local_Storage::get_program_path();
+    std::string path(get_env_variable("SteamPath"));
-    strcpy(steam_folder, path.c_str());
+    if (path.empty()) {
-    steam_folder[path.length() - 1] = 0;
+        path = Local_Storage::get_program_path();
+    }
+
+    auto count = path.copy(steam_folder, sizeof(steam_folder) - 1);
+    steam_folder[count] = '\0';
+
+    PRINT_DEBUG("returned path '%s'", steam_folder);
     return steam_folder;
 }
 
@@ -638,8 +645,7 @@ STEAMAPI_API HSteamUser GetHSteamUser()
 STEAMAPI_API steam_bool S_CALLTYPE SteamAPI_InitSafe()
 {
     PRINT_DEBUG_ENTRY();
-    SteamAPI_Init();
+    return SteamAPI_Init();
-    return true;
 }
 
 STEAMAPI_API ISteamClient *SteamClient()

dll/dll/steam_client.h

@@ -64,35 +64,6 @@ enum Steam_Pipe {
     SERVER
 };
 
-class Steam_Client;
-class Client_Background_Thread
-{
-private:
-    // don't run immediately, give the game some time to initialize
-    constexpr const static auto initial_delay = std::chrono::seconds(2);
-    // max allowed time in which RunCallbacks() might not be called
-    constexpr const static auto max_stall_ms = std::chrono::milliseconds(300);
-
-    std::thread background_keepalive{};
-    std::mutex kill_background_thread_mutex{};
-    std::condition_variable kill_background_thread_cv{};
-    bool kill_background_thread{};
-    
-    Steam_Client *client_instance{};
-    
-    void thread_proc();
-
-public:
-    Client_Background_Thread();
-    ~Client_Background_Thread();
-
-    // spawn the thread if necessary, never call this inside the ctor of Steam_Client
-    // since the thread will attempt to get the global client pointer during initialization (which will still be under construction)
-    void start(Steam_Client *client_instance);
-    // kill the thread if necessary
-    void kill();
-};
-
 class Steam_Client :
 public ISteamClient007,
 public ISteamClient008,
@@ -117,6 +88,14 @@ private:
     std::atomic_bool cb_run_active = false;
     std::atomic<unsigned long long> last_cb_run{};
 
+    // don't run immediately, give the game some time to initialize
+    constexpr const static auto initial_delay = std::chrono::seconds(2);
+    // max allowed time in which RunCallbacks() might not be called
+    constexpr const static auto max_stall_ms = std::chrono::milliseconds(300);
+
+    common_helpers::KillableWorker *background_thread{};
+    void background_thread_proc();
+
 public:
     Networking *network{};
     SteamCallResults *callback_results_server{}, *callback_results_client{};
@@ -173,8 +152,6 @@ public:
     Steam_Masterserver_Updater *steam_masterserver_updater{};
     Steam_AppTicket *steam_app_ticket{};
 
-    Client_Background_Thread *background_thread{};
-
     Steam_Overlay* steam_overlay{};
 
     bool steamclient_server_inited = false;
@@ -349,9 +326,6 @@ public:
 
     void DestroyAllInterfaces();
 
-    bool runcallbacks_active() const;
-    unsigned long long get_last_runcallbacks_time() const;
-    void set_last_runcallbacks_time(unsigned long long time_ms);
 };
 
 #endif // __INCLUDED_STEAM_CLIENT_H__

dll/steam_client.cpp

@@ -19,13 +19,33 @@
 #include "dll/settings_parser.h"
 
 
+void Steam_Client::background_thread_proc()
+{
+    auto now_ms = (unsigned long long)std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now().time_since_epoch()).count();
+
+    // if our time exceeds last run time of callbacks and it wasn't processing already
+    const auto runcallbacks_timeout_ms = last_cb_run + max_stall_ms.count();
+    if (!cb_run_active && (now_ms >= runcallbacks_timeout_ms)) {
+        std::lock_guard lock(global_mutex);
+
+        PRINT_DEBUG("run @@@@@@@@@@@@@@@@@@@@@@@@@@@");
+        last_cb_run = now_ms; // update the time counter just to avoid overlap
+        network->Run(); // networking must run first since it receives messages used by each run_callback()
+        run_every_runcb->run(); // call each run_callback()
+    }
+}
+
 Steam_Client::Steam_Client()
 {
     PRINT_DEBUG("start ----------");
     uint32 appid = create_localstorage_settings(&settings_client, &settings_server, &local_storage);
     local_storage->update_save_filenames(Local_Storage::remote_storage_folder);
 
-    background_thread = new Client_Background_Thread();
+    background_thread = new common_helpers::KillableWorker(
+        [this](void *){background_thread_proc(); return false;},
+        std::chrono::duration_cast<std::chrono::milliseconds>(initial_delay),
+        std::chrono::duration_cast<std::chrono::milliseconds>(max_stall_ms)
+    );
     network = new Networking(settings_server->get_local_steam_id(), appid, settings_server->get_port(), &(settings_server->custom_broadcasts), settings_server->disable_networking);
 
     run_every_runcb = new RunEveryRunCB();
@@ -296,6 +316,7 @@ HSteamUser Steam_Client::ConnectToGlobalUser( HSteamPipe hSteamPipe )
     // hence all run_callbacks() will never run, which might break the assumption that these callbacks are always run
     // also networking callbacks won't run
     // hence we spawn the background thread here which trigger all run_callbacks() and run networking callbacks
+    PRINT_DEBUG("started background thread");
     background_thread->start(this);
 
     steam_overlay->SetupOverlay();
@@ -317,6 +338,7 @@ HSteamUser Steam_Client::CreateLocalUser( HSteamPipe *phSteamPipe, EAccountType
     serverInit();
 
     // gameservers don't call ConnectToGlobalUser(), instead they call this function
+    PRINT_DEBUG("started background thread");
     background_thread->start(this);
 
     HSteamPipe pipe = CreateSteamPipe();
@@ -386,7 +408,10 @@ bool Steam_Client::BShutdownIfAllPipesClosed()
     PRINT_DEBUG_ENTRY();
     if (steam_pipes.size()) return false; // not all pipes are released via BReleaseSteamPipe() yet
     
+    PRINT_DEBUG("killing background thread...");
     background_thread->kill();
+    PRINT_DEBUG("killed background thread");
+
     steam_controller->Shutdown();
     steam_overlay->UnSetupOverlay();
 
@@ -928,18 +953,3 @@ void Steam_Client::DestroyAllInterfaces()
 {
     PRINT_DEBUG_TODO();
 }
-
-bool Steam_Client::runcallbacks_active() const
-{
-    return cb_run_active;
-}
-
-unsigned long long Steam_Client::get_last_runcallbacks_time() const
-{
-    return last_cb_run;
-}
-
-void Steam_Client::set_last_runcallbacks_time(unsigned long long time_ms)
-{
-    last_cb_run = time_ms;
-}

dll/steam_client_background_thread.cpp

@@ -1,90 +0,0 @@
-/* Copyright (C) 2019 Mr Goldberg
-   This file is part of the Goldberg Emulator
-
-   The Goldberg Emulator is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 3 of the License, or (at your option) any later version.
-
-   The Goldberg Emulator is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the Goldberg Emulator; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include "dll/steam_client.h"
-#include "dll/dll.h"
-
-void Client_Background_Thread::thread_proc()
-{
-    // wait for some time
-    {
-
-        std::unique_lock lck(kill_background_thread_mutex);
-        if (kill_background_thread_cv.wait_for(lck, initial_delay, [&] { return kill_background_thread; })) {
-            PRINT_DEBUG("early exit");
-            return;
-        }
-    }
-
-    PRINT_DEBUG("starting");
-
-    while (1) {
-        {
-            std::unique_lock lck(kill_background_thread_mutex);
-            if (kill_background_thread_cv.wait_for(lck, max_stall_ms, [&] { return kill_background_thread; })) {
-                PRINT_DEBUG("exit");
-                return;
-            }
-        }
-
-        auto now_ms = (unsigned long long)std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now().time_since_epoch()).count();
-
-        // if our time exceeds last run time of callbacks and it wasn't processing already
-        const auto runcallbacks_timeout_ms = client_instance->get_last_runcallbacks_time() + max_stall_ms.count();
-        if (!client_instance->runcallbacks_active() && (now_ms >= runcallbacks_timeout_ms)) {
-            std::lock_guard lock(global_mutex);
-            PRINT_DEBUG("run @@@@@@@@@@@@@@@@@@@@@@@@@@@");
-            client_instance->set_last_runcallbacks_time(now_ms); // update the time counter just to avoid overlap
-            client_instance->network->Run(); // networking must run first since it receives messages used by each run_callback()
-            client_instance->run_every_runcb->run(); // call each run_callback()
-        }
-    }
-}
-
-
-Client_Background_Thread::Client_Background_Thread()
-{
-
-}
-
-Client_Background_Thread::~Client_Background_Thread()
-{
-    kill();
-}
-
-void Client_Background_Thread::start(Steam_Client *client_instance)
-{
-    if (background_keepalive.joinable()) return; // alrady spawned
-    
-    this->client_instance = client_instance;
-    background_keepalive = std::thread([this] { thread_proc(); });
-    PRINT_DEBUG("spawned background thread *********");
-}
-
-void Client_Background_Thread::kill()
-{
-    if (!background_keepalive.joinable()) return; // already killed
-    
-    {
-        std::lock_guard lk(kill_background_thread_mutex);
-        kill_background_thread = true;
-    }
-    kill_background_thread_cv.notify_one();
-    PRINT_DEBUG("joining worker thread");
-    background_keepalive.join();
-    PRINT_DEBUG("worker thread killed");
-}

helpers/common_helpers.cpp

@@ -2,9 +2,94 @@
 #include <fstream>
 #include <cwchar>
 #include <algorithm>
-#include <thread>
 #include <cctype>
 
+namespace common_helpers {
+
+KillableWorker::KillableWorker(
+    std::function<bool(void *)> thread_job,
+    std::chrono::milliseconds initial_delay,
+    std::chrono::milliseconds polling_time,
+    std::function<bool()> should_kill)
+{
+    this->thread_job = thread_job;
+    this->initial_delay = initial_delay;
+    this->polling_time = polling_time;
+    this->should_kill = should_kill;
+}
+
+KillableWorker::~KillableWorker()
+{
+    kill();
+}
+
+KillableWorker& KillableWorker::operator=(const KillableWorker &other)
+{
+    if (&other == this) {
+        return *this;
+    }
+    
+    kill();
+
+    thread_obj = {};
+    initial_delay = other.initial_delay;
+    polling_time = other.polling_time;
+    should_kill = other.should_kill;
+    thread_job = other.thread_job;
+
+    return *this;
+}
+
+void KillableWorker::thread_proc(void *data)
+{
+    // wait for some time
+    if (initial_delay.count() > 0) {
+        std::unique_lock lck(kill_thread_mutex);
+        if (kill_thread_cv.wait_for(lck, initial_delay, [this]{ return this->kill_thread || (this->should_kill && this->should_kill()); })) {
+            return;
+        }
+    }
+
+    while (1) {
+        if (polling_time.count() > 0) {
+            std::unique_lock lck(kill_thread_mutex);
+            if (kill_thread_cv.wait_for(lck, polling_time, [this]{ return this->kill_thread || (this->should_kill && this->should_kill()); })) {
+                return;
+            }
+        }
+
+        if (thread_job(data)) { // job is done
+            return;
+        }
+        
+    }
+}
+
+bool KillableWorker::start(void *data)
+{
+    if (!thread_job) return false; // no work to do
+    if (thread_obj.joinable()) return true; // alrady spawned
+    
+    kill_thread = false;
+    thread_obj = std::thread([this, data] { thread_proc(data); });
+    return true;
+}
+
+void KillableWorker::kill()
+{
+    if (!thread_job || !thread_obj.joinable()) return; // already killed
+    
+    {
+        std::lock_guard lk(kill_thread_mutex);
+        kill_thread = true;
+    }
+
+    kill_thread_cv.notify_one();
+    thread_obj.join();
+}
+
+}
+
 static bool create_dir_impl(std::filesystem::path &dirpath)
 {
     if (std::filesystem::is_directory(dirpath))

helpers/common_helpers/common_helpers.hpp

@@ -7,9 +7,49 @@
 #include <filesystem>
 #include <chrono>
 #include <vector>
+#include <functional>
+#include <thread>
+#include <mutex>
+#include <condition_variable>
 
 namespace common_helpers {
 
+class KillableWorker
+{
+private:
+    std::thread thread_obj{};
+
+    // don't run immediately, wait some time
+    std::chrono::milliseconds initial_delay{};
+    // time between each invokation
+    std::chrono::milliseconds polling_time{};
+
+    std::function<bool()> should_kill{};
+
+    std::function<bool(void *)> thread_job;
+
+    std::mutex kill_thread_mutex{};
+    std::condition_variable kill_thread_cv{};
+    bool kill_thread{};
+    
+    void thread_proc(void *data);
+
+public:
+    KillableWorker(
+        std::function<bool(void *)> thread_proc = {},
+        std::chrono::milliseconds initial_delay = {},
+        std::chrono::milliseconds polling_time = {},
+        std::function<bool()> should_kill = {});
+    ~KillableWorker();
+
+    KillableWorker& operator=(const KillableWorker &other);
+
+    // spawn the thread if necessary
+    bool start(void *data = nullptr);
+    // kill the thread if necessary
+    void kill();
+};
+
 bool create_dir(const std::string_view &dir);
 bool create_dir(const std::wstring_view &dir);