DragonX/ObsidianDragon
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

fix(daemon): harden startup process lifecycle (crash race, exec failure, datadir lock)

Browse Source 
Three verified daemon-startup edge-case fixes in the embedded-daemon process
lifecycle (all in embedded_daemon.{cpp,h}):

- F1: EmbeddedDaemon::isRunning() (POSIX) now reads the atomic state_ instead of
  calling waitpid(WNOHANG) from the UI thread, which raced monitorProcess()'s own
  reap. waitpid is one-shot: whichever thread won consumed the exit status; if
  isRunning() won, the monitor never saw the crash, so crash_count_/State::Error
  and the 3-strike restart cap were silently lost. monitorProcess() is now the sole
  reaper (predicate Running || Stopping keeps stop()'s wait loops correct). Mirrors
  the existing XmrigManager::isRunning() fix.

- F2: startProcess() (POSIX) adds a close-on-exec self-pipe exec handshake. On a
  non-executable / wrong-arch / corrupt binary, execv fails in the child and the
  parent now learns synchronously (reads errno vs EOF), reaps the zombie, sets a
  precise last_error_ ("not executable or wrong architecture"), and returns false
  -- instead of reporting State::Running for a daemon that never started. Uses
  pipe()+FD_CLOEXEC (not pipe2) so the branch stays shared with macOS. Parent-side
  setpgid is now best-effort + logged.

- F4: start() gates on a lingering datadir lock after the port check. A graceful
  shutdown releases the RPC port ~90s before the datadir .lock, so a rapid
  stop->start spawned a daemon that died on the lock and, three times in ~12s,
  tripped the 3-strike crash cap before the lock cleared. start() now polls
  isDaemonProcessRunning() with a bounded ~300ms wait and bails with a distinct
  non-crash Error (no crash_count_ bump) that the connect loop retries once the
  lock clears. Isolated migrate-to-seed starts (skip_port_check_ / -datadir
  override) are exempt.

Adds the testDatadirLockGate unit test (pure evaluateDatadirLockGate matrix) to
test_phase4.cpp. Plan and progress tracked in docs/daemon-startup-hardening.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
DanS
2026-08-02 10:32:30 -05:00
parent 45b652f514
commit b3444e0a89
4 changed files with 655 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

129
src/daemon/embedded_daemon.cpp
View File

@@ -488,6 +488,34 @@ bool EmbeddedDaemon::start(const std::string& binary_path)
return false;
}
external_daemon_detected_ = false;
// A previous dragonxd can release the RPC port well before it releases the datadir
// .lock — a graceful shutdown can take up to ~90s (see isDaemonProcessRunning). Starting
// into a still-held lock spawns a process that dies instantly with "Cannot obtain a lock
// on data directory"; the crash monitor reports that generically and, three times in
// ~12s, that is enough to trip the 3-strike restart cap before the lock's ~90s life
// elapses. Gate on the process actually still being alive, with a SHORT bounded wait
// (not the full ~90s — start() runs on the UI thread). Isolated starts (migrate-to-seed:
// skip_port_check_ / -datadir override) are exempt; they run their own datadir+port.
{
constexpr int kDatadirLockWaitPollMs = 100;
constexpr int kDatadirLockWaitMaxPolls = 3; // ~300ms total, breaks early on exit
bool stillRunning = false;
if (!skip_port_check_ && override_datadir_.empty()) {
stillRunning = isDaemonProcessRunning();
for (int i = 0; stillRunning && i < kDatadirLockWaitMaxPolls; ++i) {
std::this_thread::sleep_for(std::chrono::milliseconds(kDatadirLockWaitPollMs));
stillRunning = isDaemonProcessRunning();
}
}
const StartLockGateDecision gate =
evaluateDatadirLockGate(skip_port_check_, !override_datadir_.empty(), stillRunning);
if (!gate.proceed) {
VERBOSE_LOGF("[INFO] %s\n", gate.errorMessage);
setState(State::Error, gate.errorMessage);
return false;
}
}
setState(State::Starting, "Looking for dragonxd binary...");
@@ -962,18 +990,38 @@ bool EmbeddedDaemon::startProcess(const std::string& binary_path, const std::vec
last_error_ = "Failed to create pipe: " + std::string(strerror(errno));
return false;
}
// Self-pipe used purely as an exec-success/failure handshake, separate from
// the stdout pipe above. Both ends are close-on-exec, so a successful execv()
// closes the write end for free (parent reads EOF); on execv() failure the
// child writes errno here, so the parent learns synchronously instead of
// reporting State::Running for a child that never became dragonxd. We use
// pipe()+FD_CLOEXEC (not pipe2) because this POSIX branch is shared with
// macOS, which has no pipe2().
int execpipe[2];
if (pipe(execpipe) == -1) {
last_error_ = "Failed to create exec-status pipe: " + std::string(strerror(errno));
close(pipefd[0]);
close(pipefd[1]);
return false;
}
fcntl(execpipe[0], F_SETFD, FD_CLOEXEC);
fcntl(execpipe[1], F_SETFD, FD_CLOEXEC);
pid_t pid = fork();
if (pid == -1) {
last_error_ = "Fork failed: " + std::string(strerror(errno));
close(pipefd[0]);
close(pipefd[1]);
close(execpipe[0]);
close(execpipe[1]);
return false;
}
if (pid == 0) {
// Child process
close(pipefd[0]); // Close read end
close(pipefd[0]); // Close read end of the stdout pipe
close(execpipe[0]); // Child only writes the exec-status pipe
// Put child in its own process group so we can kill the entire
// group later (including dragonxd spawned by a wrapper script).
@@ -1040,22 +1088,61 @@ bool EmbeddedDaemon::startProcess(const std::string& binary_path, const std::vec
execv(binary_path.c_str(), argv.data());
}
// If we get here, exec failed
fprintf(stderr, "execv failed: %s\n", strerror(errno));
// If we get here, execv() failed — the child never became dragonxd.
// Capture errno before fprintf/strerror can clobber it, report it to
// the parent over the exec-status pipe (EINTR-safe), then exit.
int exec_errno = errno;
fprintf(stderr, "execv failed: %s\n", strerror(exec_errno));
ssize_t w;
do {
w = write(execpipe[1], &exec_errno, sizeof(exec_errno));
} while (w < 0 && errno == EINTR);
_exit(127);
}
// Parent process
close(pipefd[1]); // Close write end
close(pipefd[1]); // Close our copy of the stdout write end
close(execpipe[1]); // Must close our copy, or the read() below never sees EOF
// Exec-status handshake: EOF => execv() succeeded (its write end was closed
// on exec); a full sizeof(int) => execv() failed and the child sent errno.
int child_errno = 0;
size_t got = 0;
char* ep = reinterpret_cast<char*>(&child_errno);
for (;;) {
ssize_t n = read(execpipe[0], ep + got, sizeof(child_errno) - got);
if (n == 0) break; // EOF: exec succeeded
if (n < 0) { if (errno == EINTR) continue; break; } // other error: assume success
got += static_cast<size_t>(n);
if (got >= sizeof(child_errno)) break; // full errno: exec failed
}
close(execpipe[0]);
if (got >= sizeof(child_errno)) {
// execv() never replaced the child; it fprintf'd and _exit(127)'d. Reap
// the already-dead zombie here — monitorProcess() is only started after
// this function returns true, so there is no competing reaper.
close(pipefd[0]);
int status;
waitpid(pid, &status, 0);
last_error_ = "dragonxd could not be executed: " + std::string(strerror(child_errno)) +
" — not executable or wrong architecture";
return false;
}
stdout_fd_ = pipefd[0];
// Also set process group from parent side (race with child's setpgid)
setpgid(pid, pid);
// Best-effort: the child already calls setpgid(0, 0); this parent-side call
// just closes the fork/exec race window. A failure here is not fatal to
// startup, so we log rather than abort.
if (setpgid(pid, pid) != 0) {
DEBUG_LOGF("[WARN] setpgid(%d) from parent failed: %s\n", (int)pid, strerror(errno));
}
// Set non-blocking
int flags = fcntl(stdout_fd_, F_GETFL, 0);
fcntl(stdout_fd_, F_SETFL, flags | O_NONBLOCK);
process_pid_ = pid;
return true;
}
@@ -1135,17 +1222,21 @@ double EmbeddedDaemon::getMemoryUsageMB() const
bool EmbeddedDaemon::isRunning() const
{
// Read the atomic state_ instead of calling waitpid() here. monitorProcess()
// is the sole thread allowed to waitpid() process_pid_ during normal operation.
// Calling waitpid() from this method too (as it used to, and this is invoked
// from the UI thread nearly every frame) meant whichever thread reaped the
// child's exit first consumed the status; if isRunning() won that race,
// monitorProcess() never saw the exit, so crash_count_ / the decoded exit
// code / the State::Error transition were all silently lost. Mirrors the
// fix already in XmrigManager::isRunning().
if (process_pid_ <= 0) return false;
int status;
pid_t result = waitpid(process_pid_, &status, WNOHANG);
if (result == 0) {
// Still running
return true;
}
return false;
const State s = state_.load(std::memory_order_relaxed);
// State::Stopping is included: stop()'s graceful/SIGTERM wait loops poll
// isRunning() while state_ == Stopping — before the process has actually
// terminated — and must keep seeing "alive" to wait/escalate correctly.
return (s == State::Running || s == State::Stopping);
}
void EmbeddedDaemon::drainOutput()

26
src/daemon/embedded_daemon.h
View File

@@ -235,6 +235,32 @@ public:
*/
static bool isDaemonProcessRunning();
/** Decision returned by evaluateDatadirLockGate(): whether start() may spawn now. */
struct StartLockGateDecision {
bool proceed = true; // false => bail before spawning
const char* errorMessage = ""; // set (a string literal) when proceed == false
};
/**
* @brief Pure decision for start(): bail because a previous dragonxd still holds the
* shared datadir lock? Isolated instances (skip_port_check_ / an active -datadir
* override) are exempt — they run their own throwaway datadir+port and can coexist
* with the main daemon. Does no process/fs I/O itself (the caller does the probing),
* so it is directly unit-testable; defined inline so tests need only this header.
*/
static StartLockGateDecision evaluateDatadirLockGate(bool skipPortCheck,
bool isolatedOverride,
bool stillRunningAfterWait)
{
if (skipPortCheck || isolatedOverride) return {true, ""};
if (stillRunningAfterWait) {
return {false,
"A previous dragonxd is still shutting down and holding the data "
"directory lock. Retrying shortly…"};
}
return {true, ""};
}
/** @brief Is an arbitrary TCP port currently in use on localhost? (used to pick a free port) */
static bool tcpPortInUse(int port);