Merge "Split ignoreException to avoid suppressing CTRL-C" into main

28 files changed, 76 insertions, 43 deletions

diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc
index 83bfd4fb0..e7336c7e8 100644
--- a/src/libexpr/eval-cache.cc
+++ b/src/libexpr/eval-cache.cc
@@ -79,7 +79,7 @@ struct AttrDb
                 state->txn->commit();
             state->txn.reset();
         } catch (...) {
-            ignoreException();
+            ignoreExceptionInDestructor();
         }
     }
 
@@ -90,7 +90,7 @@ struct AttrDb
         try {
             return fun();
         } catch (SQLiteError &) {
-            ignoreException();
+            ignoreExceptionExceptInterrupt();
             failed = true;
             return 0;
         }
@@ -329,7 +329,7 @@ static std::shared_ptr<AttrDb> makeAttrDb(
     try {
         return std::make_shared<AttrDb>(cfg, fingerprint, symbols);
     } catch (SQLiteError &) {
-        ignoreException();
+        ignoreExceptionExceptInterrupt();
         return nullptr;
     }
 }
diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc
index 64bd00606..029b457b1 100644
--- a/src/libmain/shared.cc
+++ b/src/libmain/shared.cc
@@ -395,7 +395,7 @@ RunPager::~RunPager()
             pid.wait();
         }
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index d90a8639d..60389fdd5 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -107,7 +107,7 @@ DerivationGoal::~DerivationGoal() noexcept(false)
 {
     /* Careful: we should never ever throw an exception from a
        destructor. */
-    try { closeLogFile(); } catch (...) { ignoreException(); }
+    try { closeLogFile(); } catch (...) { ignoreExceptionInDestructor(); }
 }
 
 
@@ -863,7 +863,7 @@ void replaceValidPath(const Path & storePath, const Path & tmpPath)
             // attempt to recover
             movePath(oldPath, storePath);
         } catch (...) {
-            ignoreException();
+            ignoreExceptionExceptInterrupt();
         }
         throw;
     }
diff --git a/src/libstore/build/hook-instance.cc b/src/libstore/build/hook-instance.cc
index f91a904cc..521f34917 100644
--- a/src/libstore/build/hook-instance.cc
+++ b/src/libstore/build/hook-instance.cc
@@ -1,4 +1,5 @@
 #include "child.hh"
+#include "error.hh"
 #include "file-system.hh"
 #include "globals.hh"
 #include "hook-instance.hh"
@@ -86,7 +87,7 @@ HookInstance::~HookInstance()
         toHook.reset();
         if (pid) pid.kill();
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc
index f3d0bc8b4..5c1fcf1f8 100644
--- a/src/libstore/build/local-derivation-goal.cc
+++ b/src/libstore/build/local-derivation-goal.cc
@@ -1,4 +1,5 @@
 #include "local-derivation-goal.hh"
+#include "error.hh"
 #include "indirect-root-store.hh"
 #include "machines.hh"
 #include "store-api.hh"
@@ -98,9 +99,9 @@ LocalDerivationGoal::~LocalDerivationGoal() noexcept(false)
 {
     /* Careful: we should never ever throw an exception from a
        destructor. */
-    try { deleteTmpDir(false); } catch (...) { ignoreException(); }
-    try { killChild(); } catch (...) { ignoreException(); }
-    try { stopDaemon(); } catch (...) { ignoreException(); }
+    try { deleteTmpDir(false); } catch (...) { ignoreExceptionInDestructor(); }
+    try { killChild(); } catch (...) { ignoreExceptionInDestructor(); }
+    try { stopDaemon(); } catch (...) { ignoreExceptionInDestructor(); }
 }
 
 
@@ -1249,7 +1250,7 @@ void LocalDerivationGoal::startDaemon()
                         NotTrusted, daemon::Recursive);
                     debug("terminated daemon connection");
                 } catch (SysError &) {
-                    ignoreException();
+                    ignoreExceptionExceptInterrupt();
                 }
             });
 
diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc
index 8088bf668..6dddf6e40 100644
--- a/src/libstore/build/substitution-goal.cc
+++ b/src/libstore/build/substitution-goal.cc
@@ -291,7 +291,7 @@ void PathSubstitutionGoal::cleanup()
             thr.get();
         }
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc
index 10c810e49..6fe1f9a05 100644
--- a/src/libstore/filetransfer.cc
+++ b/src/libstore/filetransfer.cc
@@ -115,7 +115,7 @@ struct curlFileTransfer : public FileTransfer
                 if (!done)
                     fail(FileTransferError(Interrupted, {}, "download of '%s' was interrupted", request.uri));
             } catch (...) {
-                ignoreException();
+                ignoreExceptionInDestructor();
             }
         }
 
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index d5903d01e..99bf80994 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -923,8 +923,8 @@ void LocalStore::autoGC(bool sync)
 
             } catch (...) {
                 // FIXME: we could propagate the exception to the
-                // future, but we don't really care.
-                ignoreException();
+                // future, but we don't really care. (what??)
+                ignoreExceptionInDestructor();
             }
 
         }).detach();
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 1af0f54de..c3248c2c3 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -481,7 +481,7 @@ LocalStore::~LocalStore()
             unlink(fnTempRoots.c_str());
         }
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
@@ -1222,7 +1222,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
             try {
                 parseDump(sink, source);
             } catch (...) {
-                ignoreException();
+                ignoreExceptionExceptInterrupt();
             }
         }
     };
diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc
index c60e5a85d..14381b6e0 100644
--- a/src/libstore/optimise-store.cc
+++ b/src/libstore/optimise-store.cc
@@ -31,7 +31,7 @@ struct MakeReadOnly
             /* This will make the path read-only. */
             if (path != "") canonicaliseTimestampAndPermissions(path);
         } catch (...) {
-            ignoreException();
+            ignoreExceptionInDestructor();
         }
     }
 };
diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc
index ced0f30bb..3225857ec 100644
--- a/src/libstore/pathlocks.cc
+++ b/src/libstore/pathlocks.cc
@@ -145,7 +145,7 @@ PathLocks::~PathLocks()
     try {
         unlock();
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
diff --git a/src/libstore/pathlocks.hh b/src/libstore/pathlocks.hh
index d06d031b5..feb0f5548 100644
--- a/src/libstore/pathlocks.hh
+++ b/src/libstore/pathlocks.hh
@@ -1,6 +1,7 @@
 #pragma once
 ///@file
 
+#include "error.hh"
 #include "file-descriptor.hh"
 
 namespace nix {
@@ -53,7 +54,7 @@ struct FdLock
             if (acquired)
                 lockFile(fd, ltNone, false);
         } catch (SysError &) {
-            ignoreException();
+            ignoreExceptionInDestructor();
         }
     }
 };
diff --git a/src/libstore/remote-fs-accessor.cc b/src/libstore/remote-fs-accessor.cc
index 0689ce74d..59d267873 100644
--- a/src/libstore/remote-fs-accessor.cc
+++ b/src/libstore/remote-fs-accessor.cc
@@ -29,7 +29,7 @@ ref<FSAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std::str
             /* FIXME: do this asynchronously. */
             writeFile(makeCacheFile(hashPart, "nar"), nar);
         } catch (...) {
-            ignoreException();
+            ignoreExceptionExceptInterrupt();
         }
     }
 
@@ -41,7 +41,7 @@ ref<FSAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std::str
             nlohmann::json j = listNar(narAccessor, "", true);
             writeFile(makeCacheFile(hashPart, "ls"), j.dump());
         } catch (...) {
-            ignoreException();
+            ignoreExceptionExceptInterrupt();
         }
     }
 
diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index 1f94ca03f..a9f9818be 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -1,3 +1,4 @@
+#include "error.hh"
 #include "serialise.hh"
 #include "signals.hh"
 #include "path-with-outputs.hh"
@@ -855,7 +856,7 @@ RemoteStore::Connection::~Connection()
     try {
         to.flush();
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
@@ -985,7 +986,7 @@ void RemoteStore::ConnectionHandle::withFramedSink(std::function<void(Sink & sin
                 try {
                     std::rethrow_exception(ex);
                 } catch (...) {
-                    ignoreException();
+                    ignoreExceptionExceptInterrupt();
                 }
             }
         }
diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc
index 8d0bfcb11..7aa0b6629 100644
--- a/src/libstore/sqlite.cc
+++ b/src/libstore/sqlite.cc
@@ -85,7 +85,7 @@ SQLite::~SQLite()
         if (db && sqlite3_close(db) != SQLITE_OK)
             SQLiteError::throw_(db, "closing database");
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
@@ -124,7 +124,7 @@ SQLiteStmt::~SQLiteStmt()
         if (stmt && sqlite3_finalize(stmt) != SQLITE_OK)
             SQLiteError::throw_(db, "finalizing statement '%s'", sql);
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
@@ -248,7 +248,7 @@ SQLiteTxn::~SQLiteTxn()
         if (active && sqlite3_exec(db, "rollback;", 0, 0, 0) != SQLITE_OK)
             SQLiteError::throw_(db, "aborting transaction");
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index 50d392779..d35a0e6a1 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -1163,7 +1163,7 @@ std::map<StorePath, StorePath> copyPaths(
         // not be within our control to change that, and we might still want
         // to at least copy the output paths.
         if (e.missingFeature == Xp::CaDerivations)
-            ignoreException();
+            ignoreExceptionExceptInterrupt();
         else
             throw;
     }
diff --git a/src/libutil/current-process.cc b/src/libutil/current-process.cc
index 33cda211b..3b3e46a9a 100644
--- a/src/libutil/current-process.cc
+++ b/src/libutil/current-process.cc
@@ -49,7 +49,7 @@ unsigned int getMaxCPU()
         auto period = cpuMaxParts[1];
         if (quota != "max")
                 return std::ceil(std::stoi(quota) / std::stof(period));
-    } catch (Error &) { ignoreException(lvlDebug); }
+    } catch (Error &) { ignoreExceptionInDestructor(lvlDebug); }
     #endif
 
     return 0;
diff --git a/src/libutil/error.cc b/src/libutil/error.cc
index a7cbfbfd0..027f0b1a5 100644
--- a/src/libutil/error.cc
+++ b/src/libutil/error.cc
@@ -4,6 +4,7 @@
 #include "position.hh"
 #include "terminal.hh"
 #include "strings.hh"
+#include "signals.hh"
 
 #include <iostream>
 #include <optional>
@@ -416,7 +417,7 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s
     return out;
 }
 
-void ignoreException(Verbosity lvl)
+void ignoreExceptionInDestructor(Verbosity lvl)
 {
     /* Make sure no exceptions leave this function.
        printError() also throws when remote is closed. */
@@ -429,4 +430,15 @@ void ignoreException(Verbosity lvl)
     } catch (...) { }
 }
 
+void ignoreExceptionExceptInterrupt(Verbosity lvl)
+{
+    try {
+        throw;
+    } catch (const Interrupted & e) {
+        throw;
+    } catch (std::exception & e) {
+        printMsg(lvl, "error (ignored): %1%", e.what());
+    }
+}
+
 }
diff --git a/src/libutil/error.hh b/src/libutil/error.hh
index 73c1ccadd..4eff2c2bc 100644
--- a/src/libutil/error.hh
+++ b/src/libutil/error.hh
@@ -204,7 +204,22 @@ public:
 /**
  * Exception handling in destructors: print an error message, then
  * ignore the exception.
+ *
+ * If you're not in a destructor, you usually want to use `ignoreExceptionExceptInterrupt()`.
+ *
+ * This function might also be used in callbacks whose caller may not handle exceptions,
+ * but ideally we propagate the exception using an exception_ptr in such cases.
+ * See e.g. `PackBuilderContext`
+ */
+void ignoreExceptionInDestructor(Verbosity lvl = lvlError);
+
+/**
+ * Not destructor-safe.
+ * Print an error message, then ignore the exception.
+ * If the exception is an `Interrupted` exception, rethrow it.
+ *
+ * This may be used in a few places where Interrupt can't happen, but that's ok.
  */
-void ignoreException(Verbosity lvl = lvlError);
+void ignoreExceptionExceptInterrupt(Verbosity lvl = lvlError);
 
 }
diff --git a/src/libutil/file-descriptor.cc b/src/libutil/file-descriptor.cc
index 8385ea402..cbb2bb539 100644
--- a/src/libutil/file-descriptor.cc
+++ b/src/libutil/file-descriptor.cc
@@ -146,7 +146,7 @@ AutoCloseFD::~AutoCloseFD()
     try {
         close();
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc
index 1d3eba58f..c4ffb1d0c 100644
--- a/src/libutil/file-system.cc
+++ b/src/libutil/file-system.cc
@@ -522,7 +522,7 @@ AutoDelete::~AutoDelete()
             }
         }
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc
index 7d9482814..7609e6e39 100644
--- a/src/libutil/logging.cc
+++ b/src/libutil/logging.cc
@@ -352,7 +352,7 @@ Activity::~Activity()
     try {
         logger.stopActivity(id);
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc
index f509fedff..2f5a11a28 100644
--- a/src/libutil/serialise.cc
+++ b/src/libutil/serialise.cc
@@ -83,7 +83,7 @@ void BufferedSink::flush()
 
 FdSink::~FdSink()
 {
-    try { flush(); } catch (...) { ignoreException(); }
+    try { flush(); } catch (...) { ignoreExceptionInDestructor(); }
 }
 
 
diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh
index 3a9685e0e..08ea9a135 100644
--- a/src/libutil/serialise.hh
+++ b/src/libutil/serialise.hh
@@ -549,7 +549,7 @@ struct FramedSource : Source
                 }
             }
         } catch (...) {
-            ignoreException();
+            ignoreExceptionInDestructor();
         }
     }
 
@@ -595,7 +595,7 @@ struct FramedSink : nix::BufferedSink
             to << 0;
             to.flush();
         } catch (...) {
-            ignoreException();
+            ignoreExceptionInDestructor();
         }
     }
 
diff --git a/src/libutil/signals.cc b/src/libutil/signals.cc
index 04a697d01..4e9ed0ba1 100644
--- a/src/libutil/signals.cc
+++ b/src/libutil/signals.cc
@@ -78,7 +78,7 @@ void triggerInterrupt()
             try {
                 callback();
             } catch (...) {
-                ignoreException();
+                ignoreExceptionInDestructor();
             }
         }
     }
diff --git a/src/libutil/thread-pool.cc b/src/libutil/thread-pool.cc
index 0ff83e997..cd380b608 100644
--- a/src/libutil/thread-pool.cc
+++ b/src/libutil/thread-pool.cc
@@ -109,9 +109,8 @@ void ThreadPool::doWork(bool mainThread)
                         try {
                             std::rethrow_exception(exc);
                         } catch (std::exception & e) {
-                            if (!dynamic_cast<Interrupted*>(&e) &&
-                                !dynamic_cast<ThreadPoolShutDown*>(&e))
-                                ignoreException();
+                            if (!dynamic_cast<ThreadPoolShutDown*>(&e))
+                                ignoreExceptionExceptInterrupt();
                         } catch (...) {
                         }
                     }
diff --git a/src/nix/develop.cc b/src/nix/develop.cc
index fb144c904..d1615ecdc 100644
--- a/src/nix/develop.cc
+++ b/src/nix/develop.cc
@@ -639,7 +639,7 @@ struct CmdDevelop : Common, MixEnvironment
                 throw Error("package 'nixpkgs#bashInteractive' does not provide a 'bin/bash'");
 
         } catch (Error &) {
-            ignoreException();
+            ignoreExceptionExceptInterrupt();
         }
 
         // Override SHELL with the one chosen for this environment.
diff --git a/src/nix/flake.cc b/src/nix/flake.cc
index 15c393c90..0c704a995 100644
--- a/src/nix/flake.cc
+++ b/src/nix/flake.cc
@@ -16,6 +16,7 @@
 #include "eval-cache.hh"
 #include "markdown.hh"
 #include "terminal.hh"
+#include "signals.hh"
 
 #include <limits>
 #include <nlohmann/json.hpp>
@@ -367,9 +368,11 @@ struct CmdFlakeCheck : FlakeCommand
         auto reportError = [&](const Error & e) {
             try {
                 throw e;
+            } catch (Interrupted & e) {
+                throw;
             } catch (Error & e) {
                 if (settings.keepGoing) {
-                    ignoreException();
+                    ignoreExceptionExceptInterrupt();
                     hasErrors = true;
                 }
                 else