From 8f8dc4c922f0f2b5aaf501c814717164e575b337 Mon Sep 17 00:00:00 2001 From: DanS Date: Fri, 9 Oct 2026 21:09:48 -0500 Subject: [PATCH] perf(platform): launch URL and folder openers without waiting for them On Linux and macOS openUrl/openFolder spawned xdg-open/open and waited for it on the UI thread: 50-300 ms per click where the opener forks and exits, and on generic xdg-open sessions (sway, i3, WSLg, ...), which run the browser or file manager as xdg-open's foreground child, the window froze until the user closed that app. The opener is now left running and a detached thread reaps it; the click returns at once. Its stdin, stdout and stderr go to /dev/null again. That redirect was lost in the switch to posix_spawnp, so a text-mode browser fallback could take over the wallet's terminal. The result now means "launched" rather than "exited 0" (no caller reads it), and a non-zero opener exit is reported on stderr instead of in the debug log. Windows (ShellExecute, which never waited) is unchanged. Co-Authored-By: Claude Opus 5.5 --- src/util/platform.cpp | 49 +++++++++++++++++++++++------------ tests/test_phase4.cpp | 59 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 16 deletions(-) diff --git a/src/util/platform.cpp b/src/util/platform.cpp index 7d4e70c..91d62e4 100644 --- a/src/util/platform.cpp +++ b/src/util/platform.cpp @@ -12,6 +12,8 @@ #include #include #include +#include +#include // Embedded ui.toml (generated at build time) #if __has_include("ui_toml_embedded.h") @@ -79,29 +81,44 @@ bool launchOpener(const char* opener, const std::string& target) nullptr }; + // The opener must not share the wallet's stdio (a text-mode browser fallback such as lynx/w3m would + // grab the terminal); the old `sh -c '... >/dev/null 2>&1 &'` redirected these. + posix_spawn_file_actions_t fa; + const bool haveFa = (posix_spawn_file_actions_init(&fa) == 0); + if (haveFa) { + posix_spawn_file_actions_addopen(&fa, STDIN_FILENO, "/dev/null", O_RDONLY, 0); + posix_spawn_file_actions_addopen(&fa, STDOUT_FILENO, "/dev/null", O_WRONLY, 0); + posix_spawn_file_actions_addopen(&fa, STDERR_FILENO, "/dev/null", O_WRONLY, 0); + } + pid_t pid = 0; - int rc = posix_spawnp(&pid, opener, nullptr, nullptr, argv, environ); + int rc = posix_spawnp(&pid, opener, haveFa ? &fa : nullptr, nullptr, argv, environ); + if (haveFa) posix_spawn_file_actions_destroy(&fa); if (rc != 0) { DEBUG_LOGF("Failed to launch %s: %s\n", opener, std::strerror(rc)); return false; } - int status = 0; - pid_t waited = 0; - do { - waited = waitpid(pid, &status, 0); - } while (waited < 0 && errno == EINTR); - - if (waited < 0) { - DEBUG_LOGF("Failed waiting for %s launcher: %s\n", opener, std::strerror(errno)); - return false; + // Never wait on the caller (the UI thread): generic xdg-open sessions (sway/i3/WSLg/...) run the + // browser / file manager as xdg-open's foreground child, so the old waitpid() froze the window until + // the user closed that app. A detached waiter reaps it instead. `opener` is always a string literal. + // No Logger in the waiter: it can wake during static teardown (tests), where Logger is gone. + try { + std::thread([pid, opener] { + int status = 0; + pid_t waited = 0; + do { + waited = waitpid(pid, &status, 0); + } while (waited < 0 && errno == EINTR); + if (waited >= 0 && (!WIFEXITED(status) || WEXITSTATUS(status) != 0)) + std::fprintf(stderr, "Launcher %s exited with status %d\n", opener, + WIFEXITED(status) ? WEXITSTATUS(status) : -1); + }).detach(); + } catch (const std::system_error&) { + int status = 0; + (void)waitpid(pid, &status, WNOHANG); // out of threads: worst case one zombie until exit } - if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) { - DEBUG_LOGF("Launcher %s exited with status %d\n", opener, - WIFEXITED(status) ? WEXITSTATUS(status) : -1); - return false; - } - return true; + return true; // "launched"; the opener's exit status is no longer awaited (no caller reads it) } #endif diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index a5c8877..5ad27d2 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -8138,6 +8138,62 @@ void testAtomicFileWrite() removeTree(dir); } +#ifndef _WIN32 +// C218: openUrl/openFolder must return without waiting for the opener. Fake xdg-open/open that sleep 3 s +// are the ONLY entries on PATH (posix_spawnp keeps searching after EACCES, so on a noexec temp dir an +// appended real PATH would launch the real opener); the calls must return well before the 3 s. +void testOpenerDoesNotBlock() +{ + using dragonx::util::Platform; + { + std::error_code ec; + if (!fs::exists("/bin/sleep", ec)) { + // The fake openers run /bin/sleep: without it they exit at once and the timing check proves nothing. + std::printf("[skip] testOpenerDoesNotBlock (/bin/sleep not found)\n"); + EXPECT_FALSE(Platform::openUrl("file:///etc/passwd")); // scheme gate unchanged (spawns nothing) + return; + } + } + const fs::path dir = makeTempDir(); + for (const char* name : {"xdg-open", "open"}) { + const fs::path p = dir / name; + writeTestFile(p, "#!/bin/sh\necho 1 >> \"${0%/*}/started\"\n/bin/sleep 3\n"); + fs::permissions(p, fs::perms::owner_all); + } + const char* oldPath = std::getenv("PATH"); + const std::string savedPath = oldPath ? oldPath : ""; + setenv("PATH", dir.string().c_str(), 1); + + const auto t0 = std::chrono::steady_clock::now(); + const bool url = Platform::openUrl("https://dragonx.is"); + const bool folder = Platform::openFolder((dir / "sub").string()); + const double secs = secondsSince(t0); + + if (!savedPath.empty()) setenv("PATH", savedPath.c_str(), 1); else unsetenv("PATH"); + + if (!url && !folder) { + // posix_spawnp failed (e.g. the temp dir is mounted noexec): nothing was measured. + std::printf("[skip] testOpenerDoesNotBlock (cannot exec a script from %s)\n", dir.string().c_str()); + } else { + EXPECT_TRUE(url); + EXPECT_TRUE(folder); + EXPECT_TRUE(secs < 1.5); // the old waitpid() blocked >= 6 s here + } + EXPECT_FALSE(Platform::openUrl("file:///etc/passwd")); // scheme gate unchanged + // Each shell must have opened its script before it is deleted (else sh exits 2, "cannot open"): wait + // (bounded) for both "started" marks. After that the sleeping children don't need the scripts, and the + // detached waiters reap them. + const std::uintmax_t want = 2u * ((url ? 1u : 0u) + (folder ? 1u : 0u)); + for (const auto tw = std::chrono::steady_clock::now(); want > 0 && secondsSince(tw) < 2.0;) { + std::error_code ec; + const std::uintmax_t n = fs::file_size(dir / "started", ec); + if (!ec && n >= want) break; + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + removeTree(dir); +} +#endif + // Address checksum validation. Uses standard Base58Check / BIP173 Bech32 vectors — the // algorithms are chain-agnostic, so passing these means real DRGX addresses validate too. void testAddressChecksumValidation() @@ -12608,6 +12664,9 @@ int main() testCoalescingWriter(); testCoalescingWriterOrdering(); testAtomicFileWrite(); +#ifndef _WIN32 + testOpenerDoesNotBlock(); +#endif testHushChatCrypto(); testHushChatReceivePath(); testHushChatService();