From 60b3ad5d5a86945c39141265e1bdb7e68f820ad2 Mon Sep 17 00:00:00 2001
From: DanS <dan-s-dev@proton.me>
Date: Wed, 14 Oct 2026 06:26:13 -0500
Subject: [PATCH] fix(miner): close the xmrig stdout fd after the monitor join

Stopping the miner closed the stdout pipe before joining the monitor
thread, and the UI thread's stats poll also drained stdout. Either path
could read or close a descriptor the other side was still using, so a
read could land on a closed or already-reused fd.

- stop() now joins the monitor thread first and only then closes the
  process handle and the stdout pipe, so nothing is draining the pipe
  when it is closed.
- The stdout drain is serialized under its own mutex, and the UI
  thread's stats poll no longer drains stdout at all: the monitor thread
  is the sole reader. The process handle, pid and fds are atomic so they
  can be observed safely from the memory probe.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 src/daemon/xmrig_manager.cpp | 37 ++++++++++++++++++++++--------------
 src/daemon/xmrig_manager.h   |  9 +++++----
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/src/daemon/xmrig_manager.cpp b/src/daemon/xmrig_manager.cpp
index 0ef57c7..1235e76 100644
--- a/src/daemon/xmrig_manager.cpp
+++ b/src/daemon/xmrig_manager.cpp
@@ -356,12 +356,6 @@ void XmrigManager::stop(int wait_ms) {
         // Try graceful termination first
         TerminateProcess(process_handle_, 0);
         WaitForSingleObject(process_handle_, wait_ms);
-        CloseHandle(process_handle_);
-        process_handle_ = nullptr;
-    }
-    if (stdout_read_) {
-        CloseHandle(stdout_read_);
-        stdout_read_ = nullptr;
     }
 #else
     if (process_pid_ > 0) {
@@ -386,15 +380,29 @@ void XmrigManager::stop(int wait_ms) {
         }
         process_pid_ = 0;
     }
+#endif
+
+    if (monitor_thread_.joinable())
+        monitor_thread_.join();
+
+    // Close the pipe only after the monitor thread has finished with it (it
+    // drains stdout); closing earlier let read() hit a closed/reused fd.
+#ifdef _WIN32
+    if (process_handle_) {
+        CloseHandle(process_handle_);
+        process_handle_ = nullptr;
+    }
+    if (stdout_read_) {
+        CloseHandle(stdout_read_);
+        stdout_read_ = nullptr;
+    }
+#else
     if (stdout_fd_ >= 0) {
         close(stdout_fd_);
         stdout_fd_ = -1;
     }
 #endif
 
-    if (monitor_thread_.joinable())
-        monitor_thread_.join();
-
     state_ = State::Stopped;
     DEBUG_LOGF("[INFO] XmrigManager: stopped\n");
 }
@@ -475,6 +483,7 @@ double XmrigManager::getMemoryUsageMB() const {
 }
 
 void XmrigManager::drainOutput() {
+    std::lock_guard<std::mutex> io_lk(io_mutex_);
     if (!stdout_read_) return;
     char buf[4096];
     DWORD avail = 0;
@@ -565,7 +574,7 @@ double XmrigManager::getMemoryUsageMB() const {
 #ifdef __APPLE__
     // macOS: use ps to read RSS for xmrig process
     char cmd[128];
-    snprintf(cmd, sizeof(cmd), "ps -o rss= -p %d 2>/dev/null", process_pid_);
+    snprintf(cmd, sizeof(cmd), "ps -o rss= -p %d 2>/dev/null", (int)process_pid_.load(std::memory_order_relaxed));
     FILE* fp = popen(cmd, "r");
     if (!fp) return 0.0;
     char line[64];
@@ -578,7 +587,7 @@ double XmrigManager::getMemoryUsageMB() const {
     return mb;
 #else
     char path[64];
-    snprintf(path, sizeof(path), "/proc/%d/statm", process_pid_);
+    snprintf(path, sizeof(path), "/proc/%d/statm", (int)process_pid_.load(std::memory_order_relaxed));
     FILE* fp = fopen(path, "r");
     if (!fp) return 0.0;
     long dummy = 0, pages = 0;
@@ -592,6 +601,7 @@ double XmrigManager::getMemoryUsageMB() const {
 }
 
 void XmrigManager::drainOutput() {
+    std::lock_guard<std::mutex> io_lk(io_mutex_);
     if (stdout_fd_ < 0) return;
     char buf[4096];
     while (true) {
@@ -702,9 +712,8 @@ void XmrigManager::monitorProcess() {
 // ============================================================================
 
 void XmrigManager::pollStats() {
-    // No-op on UI thread — stats are fetched by the monitor thread.
-    // Just drain stdout so log lines stay fresh.
-    drainOutput();
+    // No-op on UI thread — stats and the stdout drain both run on the monitor
+    // thread (a second reader here raced stop() closing the fd).
 }
 
 void XmrigManager::fetchStatsHttp() {
diff --git a/src/daemon/xmrig_manager.h b/src/daemon/xmrig_manager.h
index 03c3014..cc240c5 100644
--- a/src/daemon/xmrig_manager.h
+++ b/src/daemon/xmrig_manager.h
@@ -177,12 +177,13 @@ private:
 
     // Process handles
 #ifdef _WIN32
-    HANDLE process_handle_ = nullptr;
-    HANDLE stdout_read_    = nullptr;
+    std::atomic<HANDLE> process_handle_{nullptr};
+    std::atomic<HANDLE> stdout_read_{nullptr};
 #else
-    pid_t process_pid_     = 0;
-    int   stdout_fd_       = -1;
+    std::atomic<pid_t> process_pid_{0};
+    std::atomic<int>   stdout_fd_{-1};
 #endif
+    std::mutex io_mutex_;   // serializes drainOutput(); lock order io_mutex_ -> output_mutex_
 
     std::thread monitor_thread_;
     std::atomic<bool> should_stop_{false};