Merge pull request #22 from Rovey/nvml-fault-restart

fix: a caught fault in an NVML call no longer freezes the app

Author: Rovey

CMakeLists.txt

@@ -8,7 +8,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
 # would load first. Static linking also drops the redistributable dependency.
 set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
 
-# AddressSanitizer build (the `asan` preset), run on core_tests in CI. It
+# AddressSanitizer build (the `asan` preset), run on the tests in CI. It
 # applies to every target: MSVC refuses to link sanitized and unsanitized
 # objects together (annotate_string/annotate_vector mismatch).
 option(GAO_ASAN "Build everything with AddressSanitizer" OFF)
@@ -154,4 +154,13 @@ add_executable(core_tests
 target_link_libraries(core_tests PRIVATE core doctest gao_warnings)
 add_test(NAME core_tests COMMAND core_tests)
 
-set_target_properties(core hw app gao GpuAutoOptimizer core_tests PROPERTIES COMPILE_WARNING_AS_ERROR ON)
+# The app layer where it can be tested without a GPU: the driver libraries are
+# stood in for, never loaded.
+add_executable(app_tests
+  tests/test_main.cpp
+  tests/test_driver_fault.cpp
+)
+target_link_libraries(app_tests PRIVATE app doctest gao_warnings)
+add_test(NAME app_tests COMMAND app_tests)
+
+set_target_properties(core hw app gao GpuAutoOptimizer core_tests app_tests PROPERTIES COMPILE_WARNING_AS_ERROR ON)

docs/development.md

@@ -123,7 +123,7 @@ The applied offset is always at least one step below the confirmed edge. Profile
 
 ## Testing
 
-`core_tests` covers `src/core/` and runs in CI on a GitHub-hosted `windows-2025` runner, once normally and once under AddressSanitizer. CI also builds both executables, so a link error in the hardware layer is caught there.
+`core_tests` covers `src/core/`, and `app_tests` the part of `src/app/` that can be tested with the driver libraries stood in for. Both run in CI on a GitHub-hosted `windows-2025` runner, once normally and once under AddressSanitizer. CI also builds both executables, so a link error in the hardware layer is caught there.
 
 The hardware layer cannot run in CI (the runner has no GPU). It is checked by hand on the reference RTX 4070 through [hardware-checks.md](hardware-checks.md); record the results there.
 

docs/hardware-checks.md

@@ -107,6 +107,8 @@ root; adjust the path for a Debug build.
 | 77 | The fan tile says who has the fans during a run | Start an All in one run from the window (elevated) and read the **Fan speed** tile in each step; then again when the run has ended | yes | step 1 (stock) and step 3 (the undervolt): `N % (driver)`; step 2 (the overclock, once the card is warm enough for its fan curve): `N % (run)`; step 4, while a fan speed is being judged: `N % (tuning)`, with N the speed the line above the tiles names, and the text not cut off; step 5 (the final test): `N % (run)`; after the run: `N % (curve)` once the app's own curve has the fans again, or `(driver)` while they are in the stop zone; never `(driver)` while the card reports a speed set by hand | Pass for who has the fans; the last wording not seen in the fan step (2026-10-08, reference RTX 4070, driver 617.14, build a4da898, started elevated by a script that took pictures of the window; no process above 0.5 % on the card's 3D engine before the start; 33 minutes). Step 1: `57 % (driver)`. Step 2, nine minutes in: `61 % (run)`. Step 3, at the end of its soak: `66 % (driver)`. Step 4: with **Fan speed 55 %: waiting for the temperature to settle** above the tiles the tile read `55 % (measur`, and with **Fan speed 52 %** `52 % (measur`: the right state and the right speed, and the word cut off at the edge of the tile. Step 5: `55 % (run)` under **Final test: everything together**. The word is now `(tuning)`, as long as `(driver)`, which fits, and a tile draws a value that is wider than itself smaller (348 unit tests pass, one of them on the length of the labels). The build with that change, started after the run with the card idle: `0 % (driver)`, the other tiles unchanged. Not seen on the card: the word `(tuning)` itself during a fan tune, the smaller drawing of a value that does not fit, and `(curve)` after a run (the fans were in their stop zone). The run itself, a second clean one: `Applied: core +135 MHz, memory +1100 MHz, power limit 105 %, 2850 MHz at 1035 mV`, `Fans: 55 % keeps the card at 67 C under full load`; score 5731 and 5781 it/s (+0.9 %), core clock 2792 and 2819 MHz, memory clock 10491 and 11601 MHz, peak temperature 67 and 71 C, fan speed at the end 72 and 55 %, power 195 and 174 W (-10.8 %); no note about a disturbed final test. Against check 76: the memory scan ended lower this time (+1100, there +1500), and the fan tune, now with four looks before a speed counts as settled, stopped at 55 % where it had gone to 52 %. |
 | 78 | A saved run switches apply-at-logon on | Elevated: `gao --boot off`, `gao --status` (`boot-apply: off`); start this build's `GpuAutoOptimizer.exe`; Optimize page: the checkbox **Keep the result applied at every logon** is shown and ticked; start a run (Cool & efficient is the shortest) and let it end; the result screen, `gao --status`, the dashboard. Again with the box unticked | yes | with the box ticked: the result screen ends `Saved, and applied at every logon from now on. There is nothing left to do.` and shows no **Apply at every logon** button; `gao --status`: `boot-apply: on (logon task registered)`, `boot copy:  up to date`; the dashboard's **Apply at logon** switch is on; the checkbox is gone from the Optimize page. With the box unticked: `Saved. It stays applied until reboot; turn on apply at logon to keep it.`, the button, and `boot-apply: off` | Pass with the box ticked; unticked not run (2026-10-08, reference RTX 4070, driver 617.14, a build of 846b48d with the checkbox, started elevated by a script that took pictures of the window and clicked in it). `gao --boot off`: `boot-apply off: task removed, installed copy removed`; `boot-apply: off`. The Optimize page showed the ticked box with `Copies the app to Program Files and starts it in the tray at logon.` next to it. Cool & efficient, 8 minutes: `Applied: power 90 %, core +0 MHz, memory +0 MHz`; headers `Before (stock, 30 s)` and `After (5 min)` with the note about the short stock run under the table; `Saved, and applied at every logon from now on. There is nothing left to do.`, and only **Revert to stock** and **Back** under it. `gao --status`: `profile: cool: power 90 %, ... saved 2026-10-08 23:14`, `boot-apply: on (logon task registered)`. The tray app started from Program Files at the end of the script applied the profile that was put back (`boot copy:  up to date`). Not done: a run with the box unticked, and a real log-off and log-on after it. |
 | 79 | The page scrolls with the mouse on any card | Start `GpuAutoOptimizer.exe` in a window that is too low for the whole dashboard (its default size is). Turn the mouse wheel with the cursor on the **All in one** card, on the **Undervolt** card, on one of the four profile cards and on the tuning card | no | the page scrolls each time, as it does with the cursor between the cards | Pass for the two cards that failed (2026-10-08, reference RTX 4070, build of 6883a39 with the fix, version 0.4.0, started by a script that turned the wheel and took pictures). Before the fix the owner found that the page did not scroll with the mouse on the All in one card or on the Undervolt card: those two are as tall as their text, such a panel can be a pixel short of its contents, and it then kept the wheel for itself. Every card now passes the wheel on. With the cursor on the All in one card, two notches down: the card's top edge went from y 522 to y 355. With the cursor on the Undervolt card, one notch up: to y 439; one notch down again: back to y 355. The other cards were not tried with the script (they scrolled before). |
+| 80 | A caught fault in an NVML call: no hang, the app restarts itself | With a local test build with a temporary patch that is never committed: `--inject-nvml-fault=S` gives one `nvmlDeviceGetTemperature` call a bad result pointer S seconds after the app started, so the call faults inside `nvml.dll` after NVML has counted it in. Not elevated, no other copy running. (a) `GpuAutoOptimizer.exe --inject-nvml-fault=8`, window open; (b) `GpuAutoOptimizer.exe --tray --inject-nvml-fault=6`; (c) `GpuAutoOptimizer.exe --resume --after-fault --hidden --inject-nvml-fault=3`. Watch the processes and their command lines | no | (a) within 2 s of the fault a new copy runs with `--resume --after-fault`, the first copy has exited with code 0 and the new one responds, window open; (b) the same with `--hidden` added and no window; (c) the copy goes on running and responding on the library it no longer shuts down, and makes way for a new one only 60 s after it started | Pass (2026-10-10, reference RTX 4070, driver 617.42, test build of c7f18c9). What it is for: on 2026-10-09 the window froze for good during an All in one run (v0.4.0 build; the NVIDIA installer had replaced driver 617.14 by 617.42 ninety seconds before the run, with the app running, and the process still had the 617.14 `nvml.dll` loaded). The driver reset at memory +2550 MHz, as it does at that edge; the card ended at stock. A dump of the frozen process showed the window's thread in `nvmlShutdown`, waiting for NVML's count of calls in flight to reach 0 while it held the lock `nvmlInit` takes, the run's thread spinning on that lock in `nvmlInit`, the count at 1 and no thread inside any NVML call: a call had been thrown out by an access violation that our guard caught. Reproduced on its own on 617.42 with a 40-line program: after a guarded `nvmlDeviceGetTemperature` with a bad result pointer, `nvmlShutdown` does not return (waited 5 s) and neither does an `nvmlInit` after it; without the fault both return at once. With the fix: (a) fault at 8 s, new copy `--resume --after-fault` at 8.3 s, first copy gone at 8.7 s with exit code 0, new copy responding with its window; (b) fault at 6 s, new copy `--resume --after-fault --hidden` at 6.4 s, no window; (c) fault at 3 s, still running and responding at 20 s, new copy at 60.7 s. Not covered here, see check 81: anything elevated (the `boot.log` line, a tune kept applied across the restart, a fault during a run). |
+| 81 | A caught fault in an NVML call, elevated: the tune stays applied, a run ends properly | With the test build of check 80, elevated, no other copy running, a saved tune applied. (a) `GpuAutoOptimizer.exe --tray --inject-nvml-fault=20`, wait 60 s, `gao --status`, read the end of `boot.log`; (b) `GpuAutoOptimizer.exe --inject-nvml-fault=120`, start a run at once and leave the window open until the run has ended, then close the window | yes | (a) `boot.log` has `a call into the NVIDIA driver library faulted; restarting the app to load the library afresh`; the tray icon is back within seconds and says `tune kept applied`; `applied:` equals the saved tune. (b) the run goes on after the fault at two minutes and ends at its result or at stock; its result stays on the screen; the app restarts only when the window is closed, and its icon is back in the tray | Not run yet. |
 
 ## Notes
 

src/app/guarded_gpu.cpp

@@ -5,20 +5,72 @@
 #include "hw/gpu_control.hpp"
 #include "hw/nvapi.hpp"
 #include "hw/nvml.hpp"
+#include <atomic>
+#include <string>
+#include <cstdio>
 #include <optional>
 #include <utility>
 
 namespace gao::app {
 
+namespace {
+// The stack at the last caught fault, taken in the exception filter: the
+// frames of the fault are still there then. Plain data, nothing allocated.
+struct FaultSite {
+    void* frames[32];
+    unsigned short count = 0;
+};
+thread_local FaultSite t_fault;
+std::atomic<void (*)(const std::string&)> g_fault_log{nullptr};
+
+int fault_filter(unsigned long code) {
+    if (code != EXCEPTION_ACCESS_VIOLATION) return EXCEPTION_CONTINUE_SEARCH;
+    t_fault.count = RtlCaptureStackBackTrace(0, 32, t_fault.frames, nullptr);
+    return EXCEPTION_EXECUTE_HANDLER;
+}
+
+void fault_log(const std::string& line) {
+    if (const auto sink = g_fault_log.load()) sink(line);
+}
+}
+
 bool guarded(void (*fn)(void*), void* ctx) {
     __try {
         fn(ctx);
         return true;
-    } __except (GetExceptionCode() == EXCEPTION_ACCESS_VIOLATION ? EXCEPTION_EXECUTE_HANDLER : EXCEPTION_CONTINUE_SEARCH) {
+    } __except (fault_filter(GetExceptionCode())) {
         return false;
     }
 }
 
+std::string last_fault() {
+    std::string out;
+    for (unsigned i = 0; i < t_fault.count; ++i) {
+        HMODULE module = nullptr;
+        char path[MAX_PATH] = "?";
+        if (GetModuleHandleExA(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
+                               static_cast<const char*>(t_fault.frames[i]), &module))
+            GetModuleFileNameA(module, path, MAX_PATH);
+        std::string name = path;
+        if (const auto slash = name.find_last_of("\\/"); slash != std::string::npos) name.erase(0, slash + 1);
+        char site[32];
+        std::snprintf(site, sizeof(site), "+0x%llx",
+                      static_cast<unsigned long long>(static_cast<const char*>(t_fault.frames[i]) - reinterpret_cast<const char*>(module)));
+        if (!out.empty()) out += " < ";
+        out += name + site;
+    }
+    return out;
+}
+
+void set_fault_log(void (*sink)(const std::string&)) { g_fault_log = sink; }
+
+bool guarded_driver(void (*fn)(void*), void* ctx) {
+    if (guarded(fn, ctx)) return true;
+    Nvml::Abandon();
+    fault_log("a call into a driver library faulted: " + last_fault());
+    return false;
+}
+
 namespace {
 constexpr int kRecoverAttempts = 15;
 constexpr DWORD kRecoverWaitMs = 2000;
@@ -32,24 +84,25 @@ GuardedGpu::~GuardedGpu() {
     inner_ = {};
     Nvml* nvml = nvml_.release();
     Nvapi* nvapi = nvapi_.release();
-    guarded([nvml] { delete nvml; });
-    guarded([nvapi] { delete nvapi; });
+    guarded_driver([nvml] { delete nvml; });
+    guarded_driver([nvapi] { delete nvapi; });
 }
 
 // Stops using the libraries and shuts them down. The only call into a library
 // that may just have faulted is its destructor, under the guard: if that
 // faults as well, the object and the loaded DLL are leaked, as in the tray's
-// hw_lost(). Leaves fresh, uninitialised objects, which answer every call
-// with a failure without reaching the driver.
+// hw_lost(). After a fault NVML's destructor no longer calls into the library
+// at all (Nvml::Abandon). Leaves fresh, uninitialised objects, which answer
+// every call with a failure without reaching the driver.
 void GuardedGpu::Disconnect() {
     live_ = false;
     inner_ = {};   // its callbacks refer to the objects deleted below
     Nvml* nvml = nvml_.release();
     Nvapi* nvapi = nvapi_.release();
     nvml_ = std::make_unique<Nvml>();
     nvapi_ = std::make_unique<Nvapi>();
-    guarded([nvml] { delete nvml; });
-    guarded([nvapi] { delete nvapi; });
+    guarded_driver([nvml] { delete nvml; });
+    guarded_driver([nvapi] { delete nvapi; });
 }
 
 // Initialises the fresh objects Disconnect (or the constructor) left and
@@ -60,14 +113,14 @@ bool GuardedGpu::Connect(std::string* why) {
         return false;
     };
     bool ok = false;
-    if (!guarded([&] { ok = nvml_->Init(); })) return fail("NVML init failed: access violation inside the driver library");
+    if (!guarded_driver([&] { ok = nvml_->Init(); })) return fail("NVML init failed: access violation inside the driver library");
     if (!ok) return fail("NVML init failed: " + nvml_->Error());
     ok = false;
-    if (!guarded([&] { ok = nvapi_->Init(); })) return fail("NVAPI init failed: access violation inside the driver library");
+    if (!guarded_driver([&] { ok = nvapi_->Init(); })) return fail("NVAPI init failed: access violation inside the driver library");
     if (!ok) return fail("NVAPI init failed: " + nvapi_->Error());
     // make_gpu_control asks NVML what the card supports: driver calls as well.
     GpuControl built;
-    if (!guarded([&] { built = make_gpu_control(*nvml_, *nvapi_, gpu_); }))
+    if (!guarded_driver([&] { built = make_gpu_control(*nvml_, *nvapi_, gpu_); }))
         return fail("NVML init failed: access violation while querying the card");
     inner_ = std::move(built);
     live_ = true;
@@ -81,7 +134,7 @@ template <class R, class Fn> R GuardedGpu::Call(R fail, Fn fn) {
     const std::lock_guard<std::mutex> lock(mutex_);
     if (!live_) return fail;
     R out = fail;
-    if (guarded([&] { out = fn(); })) return out;
+    if (guarded_driver([&] { out = fn(); })) return out;
     live_ = false;
     inner_ = {};   // nothing calls into the faulted libraries again; recover deletes them
     return fail;
@@ -170,7 +223,7 @@ bool GuardedGpu::Init(std::string* why) {
 bool GuardedGpu::FanAuto() {
     const std::lock_guard<std::mutex> lock(mutex_);
     bool ok = false;
-    if (live_ && !guarded([&] { ok = inner_.set_fan_auto && inner_.set_fan_auto(); })) {
+    if (live_ && !guarded_driver([&] { ok = inner_.set_fan_auto && inner_.set_fan_auto(); })) {
         ok = false;
         live_ = false;
         inner_ = {};   // as in Call: the faulted libraries are not called again
@@ -185,7 +238,7 @@ bool GuardedGpu::FanAuto() {
 bool GuardedGpu::DeliverFanAuto() {
     if (!fan_auto_owed_ || !inner_.set_fan_auto) return true;
     bool ok = false;
-    if (!guarded([&] { ok = inner_.set_fan_auto(); })) return false;
+    if (!guarded_driver([&] { ok = inner_.set_fan_auto(); })) return false;
     if (ok) fan_auto_owed_ = false;   // still owed otherwise; the next recover tries again
     return true;
 }
@@ -204,13 +257,24 @@ bool GuardedGpu::Recover() {
         const std::lock_guard<std::mutex> lock(mutex_);
         Disconnect();
     }
+    std::string why;   // of the last attempt
     for (int attempt = 0; attempt < kRecoverAttempts; ++attempt) {
         Sleep(kRecoverWaitMs);   // before the first attempt too: the driver needs a moment
         const std::lock_guard<std::mutex> lock(mutex_);
         Telemetry t;
-        if (Connect(nullptr) && inner_.read && guarded([&] { t = inner_.read(); }) && t.ok && DeliverFanAuto()) return true;
+        if (!Connect(&why)) {
+        } else if (!inner_.read || !guarded_driver([&] { t = inner_.read(); })) {
+            why = "the telemetry read faulted";
+        } else if (!t.ok) {
+            why = "NVML and NVAPI initialised, but the card's clock or temperature could not be read";
+        } else if (!DeliverFanAuto()) {
+            why = "handing the fans back to the driver faulted";
+        } else {
+            return true;
+        }
         Disconnect();   // not back yet; never reuse a half-initialised library
     }
+    fault_log("reconnecting to the driver failed " + std::to_string(kRecoverAttempts) + " times; the last time: " + why);
     return false;
 }
 

src/app/guarded_gpu.hpp

@@ -27,6 +27,24 @@ template <class F> bool guarded(F f) {
     return guarded([](void* p) { (*static_cast<F*>(p))(); }, &f);
 }
 
+// `guarded`, for calls into NVML or NVAPI. A fault there also marks NVML as
+// never to be shut down in this process again (Nvml::Abandon): the call that
+// faulted never left the library, and its shutdown would wait for it for
+// ever. Which of the two libraries faulted is not known here; leaving NVML
+// loaded when it was NVAPI costs nothing. The tray app then restarts itself.
+bool guarded_driver(void (*fn)(void*), void* ctx);
+// Where the access violation `guarded` last caught on this thread happened:
+// the faulting instruction and its callers, each as module+offset. Empty when
+// it has caught none.
+std::string last_fault();
+// Where a fault in a driver call, and a reconnect that failed, are written
+// down: one line each. Set once at startup (the apps use boot_log); without
+// it nothing is written. A caught fault otherwise leaves no trace at all.
+void set_fault_log(void (*sink)(const std::string&));
+template <class F> bool guarded_driver(F f) {
+    return guarded_driver([](void* p) { (*static_cast<F*>(p))(); }, &f);
+}
+
 // NVML and NVAPI for one GPU, behind a GpuControl whose callbacks run guarded
 // and whose `recover` re-creates both libraries.
 class GuardedGpu {