Split ignoreException for destructors or interrupt-safe

src/libexpr/eval-cache.cc

@@ -101,7 +101,7 @@ struct AttrDb
                 state->txn->commit();
             state->txn.reset();
         } catch (...) {
-            ignoreException();
+            ignoreExceptionInDestructor();
         }
     }
 
@@ -112,7 +112,7 @@ struct AttrDb
         try {
             return fun();
         } catch (SQLiteError &) {
-            ignoreException();
+            ignoreExceptionExceptInterrupt();
             failed = true;
             return 0;
         }
@@ -351,7 +351,7 @@ static std::shared_ptr<AttrDb> makeAttrDb(
     try {
         return std::make_shared<AttrDb>(cfg, fingerprint, symbols);
     } catch (SQLiteError &) {
-        ignoreException();
+        ignoreExceptionExceptInterrupt();
         return nullptr;
     }
 }

src/libmain/shared.cc

@@ -416,7 +416,7 @@ RunPager::~RunPager()
         }
 #endif
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 

src/libstore/build/derivation-goal.cc

@@ -90,7 +90,7 @@ DerivationGoal::~DerivationGoal()
 {
     /* Careful: we should never ever throw an exception from a
        destructor. */
-    try { closeLogFile(); } catch (...) { ignoreException(); }
+    try { closeLogFile(); } catch (...) { ignoreExceptionInDestructor(); }
 }
 
 
@@ -814,7 +814,7 @@ void replaceValidPath(const Path & storePath, const Path & tmpPath)
             // attempt to recover
             movePath(oldPath, storePath);
         } catch (...) {
-            ignoreException();
+            ignoreExceptionExceptInterrupt();
         }
         throw;
     }

src/libstore/build/substitution-goal.cc

@@ -294,7 +294,7 @@ void PathSubstitutionGoal::cleanup()
 
         outPipe.close();
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 

src/libstore/filetransfer.cc

@@ -139,7 +139,7 @@ struct curlFileTransfer : public FileTransfer
                 if (!done)
                     fail(FileTransferError(Interrupted, {}, "download of '%s' was interrupted", request.uri));
             } catch (...) {
-                ignoreException();
+                ignoreExceptionInDestructor();
             }
         }
 

src/libstore/gc.cc

@@ -958,8 +958,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();

src/libstore/local-store.cc

@@ -522,7 +522,7 @@ LocalStore::~LocalStore()
             unlink(fnTempRoots.c_str());
         }
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
@@ -1096,108 +1096,114 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
     if (checkSigs && pathInfoIsUntrusted(info))
         throw Error("cannot add path '%s' because it lacks a signature by a trusted key", printStorePath(info.path));
 
-    /* In case we are not interested in reading the NAR: discard it. */
-    bool narRead = false;
-    Finally cleanup = [&]() {
-        if (!narRead) {
-            NullFileSystemObjectSink sink;
-            try {
-                parseDump(sink, source);
-            } catch (...) {
-                ignoreException();
+    {
+        /* In case we are not interested in reading the NAR: discard it. */
+        bool narRead = false;
+        Finally cleanup = [&]() {
+            if (!narRead) {
+                NullFileSystemObjectSink sink;
+                try {
+                    parseDump(sink, source);
+                } catch (...) {
+                    // TODO: should Interrupted be handled here?
+                    ignoreExceptionInDestructor();
+                }
             }
-        }
-    };
-
-    addTempRoot(info.path);
-
-    if (repair || !isValidPath(info.path)) {
-
-        PathLocks outputLock;
-
-        auto realPath = Store::toRealPath(info.path);
+        };
 
-        /* Lock the output path.  But don't lock if we're being called
-           from a build hook (whose parent process already acquired a
-           lock on this path). */
-        if (!locksHeld.count(printStorePath(info.path)))
-            outputLock.lockPaths({realPath});
+        addTempRoot(info.path);
 
         if (repair || !isValidPath(info.path)) {
 
-            deletePath(realPath);
-
-            /* While restoring the path from the NAR, compute the hash
-               of the NAR. */
-            HashSink hashSink(HashAlgorithm::SHA256);
-
-            TeeSource wrapperSource { source, hashSink };
-
-            narRead = true;
-            restorePath(realPath, wrapperSource, settings.fsyncStorePaths);
-
-            auto hashResult = hashSink.finish();
-
-            if (hashResult.first != info.narHash)
-                throw Error("hash mismatch importing path '%s';\n  specified: %s\n  got:       %s",
-                            printStorePath(info.path), info.narHash.to_string(HashFormat::Nix32, true), hashResult.first.to_string(HashFormat::Nix32, true));
-
-            if (hashResult.second != info.narSize)
-                throw Error("size mismatch importing path '%s';\n  specified: %s\n  got:       %s",
-                    printStorePath(info.path), info.narSize, hashResult.second);
-
-            if (info.ca) {
-                auto & specified = *info.ca;
-                auto actualHash = ({
-                    auto accessor = getFSAccessor(false);
-                    CanonPath path { printStorePath(info.path) };
-                    Hash h { HashAlgorithm::SHA256 }; // throwaway def to appease C++
-                    auto fim = specified.method.getFileIngestionMethod();
-                    switch (fim) {
-                    case FileIngestionMethod::Flat:
-                    case FileIngestionMethod::NixArchive:
-                    {
-                        HashModuloSink caSink {
-                            specified.hash.algo,
-                            std::string { info.path.hashPart() },
+            PathLocks outputLock;
+
+            auto realPath = Store::toRealPath(info.path);
+
+            /* Lock the output path.  But don't lock if we're being called
+            from a build hook (whose parent process already acquired a
+            lock on this path). */
+            if (!locksHeld.count(printStorePath(info.path)))
+                outputLock.lockPaths({realPath});
+
+            if (repair || !isValidPath(info.path)) {
+
+                deletePath(realPath);
+
+                /* While restoring the path from the NAR, compute the hash
+                of the NAR. */
+                HashSink hashSink(HashAlgorithm::SHA256);
+
+                TeeSource wrapperSource { source, hashSink };
+
+                narRead = true;
+                restorePath(realPath, wrapperSource, settings.fsyncStorePaths);
+
+                auto hashResult = hashSink.finish();
+
+                if (hashResult.first != info.narHash)
+                    throw Error("hash mismatch importing path '%s';\n  specified: %s\n  got:       %s",
+                                printStorePath(info.path), info.narHash.to_string(HashFormat::Nix32, true), hashResult.first.to_string(HashFormat::Nix32, true));
+
+                if (hashResult.second != info.narSize)
+                    throw Error("size mismatch importing path '%s';\n  specified: %s\n  got:       %s",
+                        printStorePath(info.path), info.narSize, hashResult.second);
+
+                if (info.ca) {
+                    auto & specified = *info.ca;
+                    auto actualHash = ({
+                        auto accessor = getFSAccessor(false);
+                        CanonPath path { printStorePath(info.path) };
+                        Hash h { HashAlgorithm::SHA256 }; // throwaway def to appease C++
+                        auto fim = specified.method.getFileIngestionMethod();
+                        switch (fim) {
+                        case FileIngestionMethod::Flat:
+                        case FileIngestionMethod::NixArchive:
+                        {
+                            HashModuloSink caSink {
+                                specified.hash.algo,
+                                std::string { info.path.hashPart() },
+                            };
+                            dumpPath({accessor, path}, caSink, (FileSerialisationMethod) fim);
+                            h = caSink.finish().first;
+                            break;
+                        }
+                        case FileIngestionMethod::Git:
+                            h = git::dumpHash(specified.hash.algo, {accessor, path}).hash;
+                            break;
+                        }
+                        ContentAddress {
+                            .method = specified.method,
+                            .hash = std::move(h),
                         };
-                        dumpPath({accessor, path}, caSink, (FileSerialisationMethod) fim);
-                        h = caSink.finish().first;
-                        break;
+                    });
+                    if (specified.hash != actualHash.hash) {
+                        throw Error("ca hash mismatch importing path '%s';\n  specified: %s\n  got:       %s",
+                            printStorePath(info.path),
+                            specified.hash.to_string(HashFormat::Nix32, true),
+                            actualHash.hash.to_string(HashFormat::Nix32, true));
                     }
-                    case FileIngestionMethod::Git:
-                        h = git::dumpHash(specified.hash.algo, {accessor, path}).hash;
-                        break;
-                    }
-                    ContentAddress {
-                        .method = specified.method,
-                        .hash = std::move(h),
-                    };
-                });
-                if (specified.hash != actualHash.hash) {
-                    throw Error("ca hash mismatch importing path '%s';\n  specified: %s\n  got:       %s",
-                        printStorePath(info.path),
-                        specified.hash.to_string(HashFormat::Nix32, true),
-                        actualHash.hash.to_string(HashFormat::Nix32, true));
                 }
-            }
 
-            autoGC();
+                autoGC();
 
-            canonicalisePathMetaData(realPath);
+                canonicalisePathMetaData(realPath);
 
-            optimisePath(realPath, repair); // FIXME: combine with hashPath()
+                optimisePath(realPath, repair); // FIXME: combine with hashPath()
 
-            if (settings.fsyncStorePaths) {
-                recursiveSync(realPath);
-                syncParent(realPath);
+                if (settings.fsyncStorePaths) {
+                    recursiveSync(realPath);
+                    syncParent(realPath);
+                }
+
+                registerValidPath(info);
             }
 
-            registerValidPath(info);
+            outputLock.setDeletion(true);
         }
-
-        outputLock.setDeletion(true);
     }
+
+    // In case `cleanup` ignored an `Interrupted` exception
+    checkInterrupt();
 }
 
 

src/libstore/optimise-store.cc

@@ -35,7 +35,7 @@ struct MakeReadOnly
             /* This will make the path read-only. */
             if (path != "") canonicaliseTimestampAndPermissions(path);
         } catch (...) {
-            ignoreException();
+            ignoreExceptionInDestructor();
         }
     }
 };

src/libstore/pathlocks.cc

@@ -27,7 +27,7 @@ PathLocks::~PathLocks()
     try {
         unlock();
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 

src/libstore/remote-fs-accessor.cc

@@ -30,7 +30,7 @@ ref<SourceAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std:
             /* FIXME: do this asynchronously. */
             writeFile(makeCacheFile(hashPart, "nar"), nar);
         } catch (...) {
-            ignoreException();
+            ignoreExceptionExceptInterrupt();
         }
     }
 
@@ -42,7 +42,7 @@ ref<SourceAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std:
             nlohmann::json j = listNar(narAccessor, CanonPath::root, true);
             writeFile(makeCacheFile(hashPart, "ls"), j.dump());
         } catch (...) {
-            ignoreException();
+            ignoreExceptionExceptInterrupt();
         }
     }
 

src/libstore/sqlite.cc

@@ -86,7 +86,7 @@ SQLite::~SQLite()
         if (db && sqlite3_close(db) != SQLITE_OK)
             SQLiteError::throw_(db, "closing database");
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
@@ -125,7 +125,7 @@ SQLiteStmt::~SQLiteStmt()
         if (stmt && sqlite3_finalize(stmt) != SQLITE_OK)
             SQLiteError::throw_(db, "finalizing statement '%s'", sql);
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 
@@ -240,7 +240,7 @@ SQLiteTxn::~SQLiteTxn()
         if (active && sqlite3_exec(db, "rollback;", 0, 0, 0) != SQLITE_OK)
             SQLiteError::throw_(db, "aborting transaction");
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 

src/libstore/store-api.cc

@@ -1055,7 +1055,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;
     }

src/libstore/unix/build/hook-instance.cc

@@ -91,7 +91,7 @@ HookInstance::~HookInstance()
         toHook.writeSide = -1;
         if (pid != -1) pid.kill();
     } catch (...) {
-        ignoreException();
+        ignoreExceptionInDestructor();
     }
 }
 

src/libstore/unix/build/local-derivation-goal.cc

@@ -109,9 +109,9 @@ LocalDerivationGoal::~LocalDerivationGoal()
 {
     /* 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(); }
 }
 
 
@@ -1531,7 +1531,7 @@ void LocalDerivationGoal::startDaemon()
                         NotTrusted, daemon::Recursive);
                     debug("terminated daemon connection");
                 } catch (SystemError &) {
-                    ignoreException();
+                    ignoreExceptionExceptInterrupt();
                 }
             });