fix(daemon): synchronize EmbeddedDaemon fields written across the monitor/start/stop threads

Concurrency audit: last_error_ was written unlocked at the five startProcess() error sites (while the
monitor thread could be in setState() writing it under error_mutex_) and read unlocked in start() — a
data race on a std::string. process_pid_ / stdout_fd_ (POSIX) and process_handle_ (Windows) are plain
scalars written by startProcess, the stop paths and the monitor (reap) and read by accessors.

- Route every startProcess() last_error_ write through a new locked setLastError(); read the start()
  detail via the already-locked getLastError().
- Make process_pid_ / stdout_fd_ / process_handle_ std::atomic (trivially atomic scalars/handles), so
  individual loads/stores no longer race. Process start/stop ordering and the shutdown hardening are
  unchanged — this only removes the data races, not the (already-serialized) sequencing.

Synchronization only; no fund-path call touched. Threading fix — verified by a clean build + ctest; the
race itself is runtime-only. (The Windows process_handle_ atomic is reasoning/compile-only on this Linux box.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-10-19 18:31:57 -05:00
co-authored by Claude Opus 4.8
parent 8a6d523062
commit 9b6971095c
2 changed files with 13 additions and 11 deletions
+5 -3
View File
@@ -504,6 +504,8 @@ private:
std::atomic<bool> external_daemon_detected_{false};
std::string last_error_;
mutable std::mutex error_mutex_; // protects last_error_ (written by main + monitor threads)
// Locked writer for last_error_ (startProcess() runs on the caller thread while the monitor may call setState()).
void setLastError(std::string message) { std::lock_guard<std::mutex> lk(error_mutex_); last_error_ = std::move(message); }
mutable std::mutex output_mutex_; // protects process_output_
std::string process_output_;
size_t output_total_ = 0; // bytes ever appended (the absolute end of process_output_)
@@ -511,14 +513,14 @@ private:
// Process handle
#ifdef _WIN32
HANDLE process_handle_ = nullptr;
std::atomic<HANDLE> process_handle_{nullptr}; // written by start/stop/monitor threads, read by accessors
HANDLE stdout_read_ = nullptr; // pipe handle (Linux-style pipe, unused when tailing debug.log)
std::string launch_cmd_; // full command line used to launch daemon (for diagnostics)
std::string launch_binary_; // binary path used (for diagnostics)
std::string launch_workdir_; // working directory used (for diagnostics)
#else
pid_t process_pid_ = 0;
int stdout_fd_ = -1; // the child's stdout+stderr (crash diagnostics; the log itself is debug.log)
std::atomic<pid_t> process_pid_{0}; // written by start/stop paths and the monitor (reap), read by accessors
std::atomic<int> stdout_fd_{-1}; // the child's stdout+stderr (crash diagnostics; the log itself is debug.log)
void tailDebugLog();
// Waits up to timeout_ms for `pid` to exit, draining its output; true once reaped. Only valid