diff --git a/src/nvhttp.cpp b/src/nvhttp.cpp index c2e081ab628..98f59f9028f 100644 --- a/src/nvhttp.cpp +++ b/src/nvhttp.cpp @@ -1302,6 +1302,7 @@ namespace nvhttp { */ void applist(resp_https_t response, req_https_t request) { print_req(request); + proc::refresh(config::stream.file_apps); pt::ptree tree; diff --git a/src/process.cpp b/src/process.cpp index b468ce93d1b..3ca8a718f34 100644 --- a/src/process.cpp +++ b/src/process.cpp @@ -415,6 +415,20 @@ namespace proc { return _app.name; } + void proc_t::update_apps_and_env(proc_t &&other) { + if (_app_id > 0 || placebo) { + // Preserve session-specific environment variables for the running app + for (const auto &var : _env) { + std::string name = var.get_name(); + if (name.find("SUNSHINE_APP_") == 0 || name.find("SUNSHINE_CLIENT_") == 0) { + other._env[name] = var.to_string(); + } + } + } + _env = std::move(other._env); + _apps = std::move(other._apps); + } + proc_t::~proc_t() { // It's not safe to call terminate() here because our proc_t is a static variable // that may be destroyed after the Boost loggers have been destroyed. Instead, @@ -818,12 +832,36 @@ namespace proc { /** * @brief Refresh cached platform state from the operating system. + * + * This function compares the current last modified time of the file to the time it was last parsed. + * If the file has been modified since it was last parsed (or if it has never been parsed), + * it will be re-parsed to update the app list. + * + * An optional is used rather than a bare file_time_type because value-initialized + * file_time_type{} is not a reliable "never parsed" sentinel. On some toolchains + * (e.g. MSYS2 UCRT64 GCC 16.2), file_time_type::clock::now() compares + * less-or-equal to file_time_type{}, which would cause the initial parse to be skipped. + * + * When an app is currently running, only the cached app configuration list and environment + * are updated — active process state (_app_id, _process, _process_group, etc.) is preserved + * so that the in-flight streaming session can still be terminated properly. */ void refresh(const std::string &file_name) { + static std::optional last_apps_file_update; ///< Timestamp of the last successful apps.json parse, or nullopt if never parsed. + + std::error_code ec; + auto current_time = std::filesystem::last_write_time(file_name, ec); + + // Only skip parsing when we have a known-good timestamp and the file hasn't changed + if (last_apps_file_update && !ec && current_time == *last_apps_file_update) { + return; + } + auto proc_opt = proc::parse(file_name); if (proc_opt) { - proc = std::move(*proc_opt); + proc.update_apps_and_env(std::move(*proc_opt)); + last_apps_file_update = current_time; } } } // namespace proc diff --git a/src/process.h b/src/process.h index 9b89060e5fd..26bb470a196 100644 --- a/src/process.h +++ b/src/process.h @@ -149,6 +149,26 @@ namespace proc { */ void terminate(); + /** + * @brief Update the cached app list and environment without disturbing running process state. + * + * This replaces only the application configuration data (_apps and _env) from a newly parsed + * proc_t, preserving the current _app_id, _process, _process_group, and other fields that + * track an in-flight streaming session. + * + * @param other The newly parsed proc_t whose apps and env will be moved into this instance. + */ + void update_apps_and_env(proc_t &&other); + +#ifdef SUNSHINE_TESTS + /** + * @brief Get the current process environment (strictly for unit tests). + */ + const boost::process::v1::environment &get_env() const { + return _env; + } +#endif + private: int _app_id; diff --git a/tests/unit/test_process.cpp b/tests/unit/test_process.cpp index 5d5b3c578a1..3cae66d6c1d 100644 --- a/tests/unit/test_process.cpp +++ b/tests/unit/test_process.cpp @@ -8,6 +8,10 @@ // standard includes #include #include +#include + +// lib imports +#include // local includes #include @@ -30,7 +34,7 @@ TEST(ProcessTest, PrepareCommand) { #endif } -class ProcessPNGTest: public BaseTest { +class ProcessPNGTest: public BaseTest { // NOSONAR(cpp:S3656): protected members are intentional for test fixture subclassing protected: void SetUp() override { BaseTest::SetUp(); @@ -288,3 +292,333 @@ TEST_F(ProcessPNGTest, ValidateAppImagePath_OldSteamDefault) { const std::string result = proc::validate_app_image_path("./assets/steam.png"); EXPECT_EQ(result, SUNSHINE_ASSETS_DIR "/steam.png"); } + +/** + * @brief Test fixture for proc_t::update_apps_and_env and proc::refresh. + */ +class ProcessRefreshTest: public BaseTest { // NOSONAR(cpp:S3656): protected members are intentional for test fixture subclassing +protected: + void SetUp() override { + BaseTest::SetUp(); + test_dir = fs::temp_directory_path() / "sunshine_process_refresh_test"; // NOSONAR(cpp:S5443): safe for tests + fs::create_directories(test_dir); + } + + void TearDown() override { + if (fs::exists(test_dir)) { + fs::remove_all(test_dir); + } + BaseTest::TearDown(); + } + + /** + * @brief Write a minimal valid apps.json file. + * + * @param path Target path for the JSON file. + * @param app_names List of application names to include. + */ + void writeAppsJson(const fs::path &path, const std::vector &app_names) const { + std::ofstream file(path); + file << "{\n \"env\": {},\n \"apps\": [\n"; + for (size_t i = 0; i < app_names.size(); ++i) { + file << R"( { "name": ")" << app_names[i] << R"(" })"; + if (i + 1 < app_names.size()) { + file << ","; + } + file << "\n"; + } + file << " ]\n}\n"; + file.close(); + + // Artificially advance the modification time to guarantee strict monotonicity + // across tests. This prevents the static last_apps_file_update in refresh() + // from skipping parses when tests execute rapidly in the same clock tick. + static int test_time_offset = 1; + auto new_time = fs::file_time_type::clock::now() + std::chrono::seconds(test_time_offset++); + fs::last_write_time(path, new_time); + } + + fs::path test_dir; +}; + +/** + * @brief Test fixture for testing updates to process apps and environment. + */ +class ProcessUpdateTest: public ProcessRefreshTest { // NOSONAR(cpp:S3656): protected members are intentional for test fixture subclassing +protected: + proc::proc_t saved_global_proc; + + void SetUp() override { + ProcessRefreshTest::SetUp(); + + saved_global_proc = std::move(proc::proc); + + boost::process::v1::environment env = boost::this_process::environment(); + std::vector apps_initial; + proc::ctx_t ctx; + ctx.name = "Desktop"; + ctx.id = "42"; + apps_initial.push_back(std::move(ctx)); + + proc::proc_t target(std::move(env), std::move(apps_initial)); + proc::proc = std::move(target); + + auto launch_session = std::make_shared(); + launch_session->width = 1920; + launch_session->height = 1080; + launch_session->fps = 60; + launch_session->gcmap = 0; + launch_session->enable_hdr = false; + launch_session->host_audio = false; + launch_session->enable_sops = false; + launch_session->surround_info = 2; + int rc = proc::proc.execute(42, launch_session); + ASSERT_EQ(rc, 0); + } + + void TearDown() override { + proc::proc.terminate(); + proc::proc = std::move(saved_global_proc); + ProcessRefreshTest::TearDown(); + } + + /** + * @brief Build a simulated source environment and app list for the update. + * + * @return A constructed proc_t containing the new applications and environment. + */ + proc::proc_t buildNewSource() const { + boost::process::v1::environment env2 = boost::this_process::environment(); + std::vector apps_new; + proc::ctx_t ctx_new; + ctx_new.name = "NewApp"; + ctx_new.id = "99"; + apps_new.push_back(std::move(ctx_new)); + return proc::proc_t(std::move(env2), std::move(apps_new)); + } +}; + +TEST_F(ProcessRefreshTest, UpdateAppsAndEnv_UpdatesAppsList) { + // Build an initial proc_t with one app + boost::process::v1::environment env = boost::this_process::environment(); + std::vector apps_initial; + proc::ctx_t ctx_a; + ctx_a.name = "AppA"; + ctx_a.id = "100"; + apps_initial.push_back(std::move(ctx_a)); + + proc::proc_t target(std::move(env), std::move(apps_initial)); + ASSERT_EQ(target.get_apps().size(), 1u); + EXPECT_EQ(target.get_apps()[0].name, "AppA"); + + // Build a replacement proc_t with two apps + boost::process::v1::environment env2 = boost::this_process::environment(); + std::vector apps_new; + proc::ctx_t ctx_b; + ctx_b.name = "AppB"; + ctx_b.id = "200"; + apps_new.push_back(std::move(ctx_b)); + proc::ctx_t ctx_c; + ctx_c.name = "AppC"; + ctx_c.id = "300"; + apps_new.push_back(std::move(ctx_c)); + + proc::proc_t source(std::move(env2), std::move(apps_new)); + + // Act + target.update_apps_and_env(std::move(source)); + + // Assert — apps list must now contain the new apps + ASSERT_EQ(target.get_apps().size(), 2u); + EXPECT_EQ(target.get_apps()[0].name, "AppB"); + EXPECT_EQ(target.get_apps()[1].name, "AppC"); +} + +TEST_F(ProcessUpdateTest, UpdateAppsAndEnv_PreservesRunningState) { + proc::proc.update_apps_and_env(buildNewSource()); + + ASSERT_EQ(proc::proc.get_apps().size(), 1u); + EXPECT_EQ(proc::proc.get_apps()[0].name, "NewApp"); + EXPECT_EQ(proc::proc.running(), 42) << "update_apps_and_env must not reset _app_id or placebo"; +} + +TEST_F(ProcessUpdateTest, UpdateAppsAndEnv_PreservesSessionEnvironment) { + proc::proc.update_apps_and_env(buildNewSource()); + + const auto &active_env = proc::proc.get_env(); + EXPECT_TRUE(active_env.count("SUNSHINE_APP_NAME") > 0) + << "SUNSHINE_APP_NAME was wiped from the environment during update_apps_and_env"; + + if (active_env.count("SUNSHINE_APP_NAME") > 0) { + EXPECT_EQ(active_env.at("SUNSHINE_APP_NAME").to_string(), "Desktop"); + } +} + +TEST_F(ProcessRefreshTest, Refresh_ParsesFileOnFirstCall) { + // The first call to refresh() must always parse the file, regardless + // of what file_time_type{} compares to relative to the file timestamp. + // + // NOTE: Because refresh() uses static local variables (has_parsed, last_apps_file_update), + // this test exercises the very first call in this test process. Subsequent + // tests that call refresh() share the same static state. + const fs::path apps_file = test_dir / "apps_initial.json"; + writeAppsJson(apps_file, {"TestApp1", "TestApp2"}); + + // Save and clear global proc + auto saved = std::move(proc::proc); + proc::proc = proc::proc_t {}; + + ASSERT_TRUE(proc::proc.get_apps().empty()); + + // Act + proc::refresh(apps_file.string()); + + // Assert — must have been parsed + ASSERT_EQ(proc::proc.get_apps().size(), 2u); + EXPECT_EQ(proc::proc.get_apps()[0].name, "TestApp1"); + EXPECT_EQ(proc::proc.get_apps()[1].name, "TestApp2"); + + // Cleanup + proc::proc = std::move(saved); +} + +TEST_F(ProcessRefreshTest, Refresh_SkipsUnchangedFile) { + const fs::path apps_file = test_dir / "apps_skip.json"; + writeAppsJson(apps_file, {"OriginalApp"}); + + auto saved = std::move(proc::proc); + proc::proc = proc::proc_t {}; + + // First refresh — should parse + proc::refresh(apps_file.string()); + ASSERT_EQ(proc::proc.get_apps().size(), 1u); + + // Mutate apps in-memory to detect whether the second refresh re-parses + proc::proc.get_apps()[0].name = "Mutated"; + + // Second refresh with unchanged file — should be skipped + proc::refresh(apps_file.string()); + EXPECT_EQ(proc::proc.get_apps()[0].name, "Mutated") + << "refresh() should skip re-parse when the file timestamp has not changed"; + + // Cleanup + proc::proc = std::move(saved); +} + +TEST_F(ProcessRefreshTest, Refresh_ReparseAfterFileModified) { + const fs::path apps_file = test_dir / "apps_modified.json"; + writeAppsJson(apps_file, {"BeforeEdit"}); + + auto saved = std::move(proc::proc); + proc::proc = proc::proc_t {}; + + proc::refresh(apps_file.string()); + ASSERT_EQ(proc::proc.get_apps().size(), 1u); + EXPECT_EQ(proc::proc.get_apps()[0].name, "BeforeEdit"); + + // Rewrite to ensure a new timestamp + writeAppsJson(apps_file, {"AfterEdit"}); + + proc::refresh(apps_file.string()); + ASSERT_EQ(proc::proc.get_apps().size(), 1u); + EXPECT_EQ(proc::proc.get_apps()[0].name, "AfterEdit"); + + // Cleanup + proc::proc = std::move(saved); +} + +TEST_F(ProcessRefreshTest, Refresh_ReparseAfterFileMtimeDecreased) { + const fs::path apps_file = test_dir / "apps_reverted.json"; + writeAppsJson(apps_file, {"Initial"}); + + auto saved = std::move(proc::proc); + proc::proc = proc::proc_t {}; + + proc::refresh(apps_file.string()); + ASSERT_EQ(proc::proc.get_apps().size(), 1u); + EXPECT_EQ(proc::proc.get_apps()[0].name, "Initial"); + + // Write a new file but artificially set its timestamp to the PAST + // simulating a backup restoration or git checkout + writeAppsJson(apps_file, {"OlderBackup"}); + auto past_time = fs::file_time_type::clock::now() - std::chrono::hours(1); + fs::last_write_time(apps_file, past_time); + + proc::refresh(apps_file.string()); + ASSERT_EQ(proc::proc.get_apps().size(), 1u); + EXPECT_EQ(proc::proc.get_apps()[0].name, "OlderBackup") + << "refresh() must re-parse if the modification time differs, even if it is strictly earlier"; + + proc::proc = std::move(saved); +} + +TEST_F(ProcessRefreshTest, Refresh_PreservesRunningAppDuringReparse) { + // This is the critical regression test: refreshing the app list while an + // app is running must not lose the active process state. + const fs::path apps_file = test_dir / "apps_running.json"; + writeAppsJson(apps_file, {"Desktop"}); + + auto saved = std::move(proc::proc); + proc::proc = proc::proc_t {}; + + // Initial parse + proc::refresh(apps_file.string()); + ASSERT_FALSE(proc::proc.get_apps().empty()); + + // Find the ID of "Desktop" as assigned by parse + auto desktop_id = std::stoi(proc::proc.get_apps()[0].id); + + // Execute it in placebo mode (empty cmd) + auto launch_session = std::make_shared(); + launch_session->width = 1920; + launch_session->height = 1080; + launch_session->fps = 60; + launch_session->gcmap = 0; + launch_session->enable_hdr = false; + launch_session->host_audio = false; + launch_session->enable_sops = false; + launch_session->surround_info = 2; + int rc = proc::proc.execute(desktop_id, launch_session); + ASSERT_EQ(rc, 0); + ASSERT_EQ(proc::proc.running(), desktop_id); + + // Simulate an external edit to the apps file + writeAppsJson(apps_file, {"Desktop", "NewApp"}); + + // Act — refresh while the app is "running" + proc::refresh(apps_file.string()); + + // Assert — app list is updated but the running state is preserved + ASSERT_EQ(proc::proc.get_apps().size(), 2u); + EXPECT_EQ(proc::proc.running(), desktop_id) + << "refresh() during a running session must preserve active process state"; + + // Cleanup — terminate before restoring + proc::proc.terminate(); + proc::proc = std::move(saved); +} + +TEST_F(ProcessRefreshTest, FileTimeType_DefaultValueComparison) { + // This test documents the behavior that triggered the timestamp guard + // regression. On some toolchains (MSYS2 UCRT64 GCC 16.2), file timestamps + // and clock::now() can compare less-or-equal to file_time_type{}. + // + // The fix uses std::optional instead of relying on + // file_time_type{} as a "never parsed" indicator. + const auto default_time = std::filesystem::file_time_type {}; + const auto now_time = std::filesystem::file_time_type::clock::now(); + + // We cannot assert the comparison direction since it is platform-dependent. + // Instead, just log and document both cases. + if (now_time <= default_time) { + BOOST_LOG(info) << "file_time_type::clock::now() <= file_time_type{} is TRUE on this platform " + << "(this is the UCRT64 case that requires the optional sentinel)"; + } else { + BOOST_LOG(info) << "file_time_type::clock::now() > file_time_type{} on this platform " + << "(default-init guard would have worked, but optional is still safer)"; + } + + // The actual correctness is verified by Refresh_ParsesFileOnFirstCall above — + // if that test passes, the optional fix works regardless of platform behavior. + SUCCEED(); +}