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

This commit is contained in:
Rebecca Turner 2024-10-05 17:33:00 +00:00 committed by Gerrit Code Review
commit 86b213e632
28 changed files with 76 additions and 43 deletions

View file

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

View file

@ -395,7 +395,7 @@ RunPager::~RunPager()
pid.wait(); pid.wait();
} }
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionInDestructor();
} }
} }

View file

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

View file

@ -1,4 +1,5 @@
#include "child.hh" #include "child.hh"
#include "error.hh"
#include "file-system.hh" #include "file-system.hh"
#include "globals.hh" #include "globals.hh"
#include "hook-instance.hh" #include "hook-instance.hh"
@ -86,7 +87,7 @@ HookInstance::~HookInstance()
toHook.reset(); toHook.reset();
if (pid) pid.kill(); if (pid) pid.kill();
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionInDestructor();
} }
} }

View file

@ -1,4 +1,5 @@
#include "local-derivation-goal.hh" #include "local-derivation-goal.hh"
#include "error.hh"
#include "indirect-root-store.hh" #include "indirect-root-store.hh"
#include "machines.hh" #include "machines.hh"
#include "store-api.hh" #include "store-api.hh"
@ -98,9 +99,9 @@ LocalDerivationGoal::~LocalDerivationGoal() noexcept(false)
{ {
/* Careful: we should never ever throw an exception from a /* Careful: we should never ever throw an exception from a
destructor. */ destructor. */
try { deleteTmpDir(false); } catch (...) { ignoreException(); } try { deleteTmpDir(false); } catch (...) { ignoreExceptionInDestructor(); }
try { killChild(); } catch (...) { ignoreException(); } try { killChild(); } catch (...) { ignoreExceptionInDestructor(); }
try { stopDaemon(); } catch (...) { ignoreException(); } try { stopDaemon(); } catch (...) { ignoreExceptionInDestructor(); }
} }
@ -1249,7 +1250,7 @@ void LocalDerivationGoal::startDaemon()
NotTrusted, daemon::Recursive); NotTrusted, daemon::Recursive);
debug("terminated daemon connection"); debug("terminated daemon connection");
} catch (SysError &) { } catch (SysError &) {
ignoreException(); ignoreExceptionExceptInterrupt();
} }
}); });

View file

@ -291,7 +291,7 @@ void PathSubstitutionGoal::cleanup()
thr.get(); thr.get();
} }
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionInDestructor();
} }
} }

View file

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

View file

@ -923,8 +923,8 @@ void LocalStore::autoGC(bool sync)
} catch (...) { } catch (...) {
// FIXME: we could propagate the exception to the // FIXME: we could propagate the exception to the
// future, but we don't really care. // future, but we don't really care. (what??)
ignoreException(); ignoreExceptionInDestructor();
} }
}).detach(); }).detach();

View file

@ -481,7 +481,7 @@ LocalStore::~LocalStore()
unlink(fnTempRoots.c_str()); unlink(fnTempRoots.c_str());
} }
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionInDestructor();
} }
} }
@ -1222,7 +1222,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
try { try {
parseDump(sink, source); parseDump(sink, source);
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionExceptInterrupt();
} }
} }
}; };

View file

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

View file

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

View file

@ -1,6 +1,7 @@
#pragma once #pragma once
///@file ///@file
#include "error.hh"
#include "file-descriptor.hh" #include "file-descriptor.hh"
namespace nix { namespace nix {
@ -53,7 +54,7 @@ struct FdLock
if (acquired) if (acquired)
lockFile(fd, ltNone, false); lockFile(fd, ltNone, false);
} catch (SysError &) { } catch (SysError &) {
ignoreException(); ignoreExceptionInDestructor();
} }
} }
}; };

View file

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

View file

@ -1,3 +1,4 @@
#include "error.hh"
#include "serialise.hh" #include "serialise.hh"
#include "signals.hh" #include "signals.hh"
#include "path-with-outputs.hh" #include "path-with-outputs.hh"
@ -855,7 +856,7 @@ RemoteStore::Connection::~Connection()
try { try {
to.flush(); to.flush();
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionInDestructor();
} }
} }
@ -985,7 +986,7 @@ void RemoteStore::ConnectionHandle::withFramedSink(std::function<void(Sink & sin
try { try {
std::rethrow_exception(ex); std::rethrow_exception(ex);
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionExceptInterrupt();
} }
} }
} }

View file

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

View file

@ -1163,7 +1163,7 @@ std::map<StorePath, StorePath> copyPaths(
// not be within our control to change that, and we might still want // not be within our control to change that, and we might still want
// to at least copy the output paths. // to at least copy the output paths.
if (e.missingFeature == Xp::CaDerivations) if (e.missingFeature == Xp::CaDerivations)
ignoreException(); ignoreExceptionExceptInterrupt();
else else
throw; throw;
} }

View file

@ -49,7 +49,7 @@ unsigned int getMaxCPU()
auto period = cpuMaxParts[1]; auto period = cpuMaxParts[1];
if (quota != "max") if (quota != "max")
return std::ceil(std::stoi(quota) / std::stof(period)); return std::ceil(std::stoi(quota) / std::stof(period));
} catch (Error &) { ignoreException(lvlDebug); } } catch (Error &) { ignoreExceptionInDestructor(lvlDebug); }
#endif #endif
return 0; return 0;

View file

@ -4,6 +4,7 @@
#include "position.hh" #include "position.hh"
#include "terminal.hh" #include "terminal.hh"
#include "strings.hh" #include "strings.hh"
#include "signals.hh"
#include <iostream> #include <iostream>
#include <optional> #include <optional>
@ -416,7 +417,7 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s
return out; return out;
} }
void ignoreException(Verbosity lvl) void ignoreExceptionInDestructor(Verbosity lvl)
{ {
/* Make sure no exceptions leave this function. /* Make sure no exceptions leave this function.
printError() also throws when remote is closed. */ printError() also throws when remote is closed. */
@ -429,4 +430,15 @@ void ignoreException(Verbosity lvl)
} catch (...) { } } catch (...) { }
} }
void ignoreExceptionExceptInterrupt(Verbosity lvl)
{
try {
throw;
} catch (const Interrupted & e) {
throw;
} catch (std::exception & e) {
printMsg(lvl, "error (ignored): %1%", e.what());
}
}
} }

View file

@ -204,7 +204,22 @@ public:
/** /**
* Exception handling in destructors: print an error message, then * Exception handling in destructors: print an error message, then
* ignore the exception. * 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 ignoreException(Verbosity lvl = lvlError); 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 ignoreExceptionExceptInterrupt(Verbosity lvl = lvlError);
} }

View file

@ -146,7 +146,7 @@ AutoCloseFD::~AutoCloseFD()
try { try {
close(); close();
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionInDestructor();
} }
} }

View file

@ -522,7 +522,7 @@ AutoDelete::~AutoDelete()
} }
} }
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionInDestructor();
} }
} }

View file

@ -352,7 +352,7 @@ Activity::~Activity()
try { try {
logger.stopActivity(id); logger.stopActivity(id);
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionInDestructor();
} }
} }

View file

@ -83,7 +83,7 @@ void BufferedSink::flush()
FdSink::~FdSink() FdSink::~FdSink()
{ {
try { flush(); } catch (...) { ignoreException(); } try { flush(); } catch (...) { ignoreExceptionInDestructor(); }
} }

View file

@ -549,7 +549,7 @@ struct FramedSource : Source
} }
} }
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionInDestructor();
} }
} }
@ -595,7 +595,7 @@ struct FramedSink : nix::BufferedSink
to << 0; to << 0;
to.flush(); to.flush();
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionInDestructor();
} }
} }

View file

@ -78,7 +78,7 @@ void triggerInterrupt()
try { try {
callback(); callback();
} catch (...) { } catch (...) {
ignoreException(); ignoreExceptionInDestructor();
} }
} }
} }

View file

@ -109,9 +109,8 @@ void ThreadPool::doWork(bool mainThread)
try { try {
std::rethrow_exception(exc); std::rethrow_exception(exc);
} catch (std::exception & e) { } catch (std::exception & e) {
if (!dynamic_cast<Interrupted*>(&e) && if (!dynamic_cast<ThreadPoolShutDown*>(&e))
!dynamic_cast<ThreadPoolShutDown*>(&e)) ignoreExceptionExceptInterrupt();
ignoreException();
} catch (...) { } catch (...) {
} }
} }

View file

@ -639,7 +639,7 @@ struct CmdDevelop : Common, MixEnvironment
throw Error("package 'nixpkgs#bashInteractive' does not provide a 'bin/bash'"); throw Error("package 'nixpkgs#bashInteractive' does not provide a 'bin/bash'");
} catch (Error &) { } catch (Error &) {
ignoreException(); ignoreExceptionExceptInterrupt();
} }
// Override SHELL with the one chosen for this environment. // Override SHELL with the one chosen for this environment.

View file

@ -16,6 +16,7 @@
#include "eval-cache.hh" #include "eval-cache.hh"
#include "markdown.hh" #include "markdown.hh"
#include "terminal.hh" #include "terminal.hh"
#include "signals.hh"
#include <limits> #include <limits>
#include <nlohmann/json.hpp> #include <nlohmann/json.hpp>
@ -367,9 +368,11 @@ struct CmdFlakeCheck : FlakeCommand
auto reportError = [&](const Error & e) { auto reportError = [&](const Error & e) {
try { try {
throw e; throw e;
} catch (Interrupted & e) {
throw;
} catch (Error & e) { } catch (Error & e) {
if (settings.keepGoing) { if (settings.keepGoing) {
ignoreException(); ignoreExceptionExceptInterrupt();
hasErrors = true; hasErrors = true;
} }
else else