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 <noreply@anthropic.com>
This commit is contained in:
+33
-16
@@ -12,6 +12,8 @@
|
||||
#include <fstream>
|
||||
#include <sstream>
|
||||
#include <filesystem>
|
||||
#include <system_error>
|
||||
#include <thread>
|
||||
|
||||
// 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
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user