From e291087747146ab862edf0c6a88b12f355fb497c Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Thu, 4 Apr 2024 17:31:01 +0200 Subject: [PATCH] libutil: guard Finally against invalid exception throws throwing exceptions is fine, but throwing exceptions during exception handling is hard enough to do correctly that we should just forbid it entirely out of an overabundance of caution. in cases where terminate is the correct answer the users of Finally must call it manually now. Source: https://git.lix.systems/lix-project/lix/commit/6c777476c9e97abfc5232f0707985caf6df2baea --- src/libutil/finally.hh | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/libutil/finally.hh b/src/libutil/finally.hh index f9f0195a1..bda4227e6 100644 --- a/src/libutil/finally.hh +++ b/src/libutil/finally.hh @@ -2,6 +2,8 @@ ///@file #include +#include +#include /** * A trivial class to run a function at the end of a scope. @@ -21,5 +23,25 @@ public: Finally(Finally &&other) : fun(std::move(other.fun)) { other.movedFrom = true; } - ~Finally() { if (!movedFrom) fun(); } + ~Finally() noexcept(false) + { + try { + if (!movedFrom) + fun(); + } catch (...) { + // finally may only throw an exception if exception handling is not already + // in progress. if handling *is* in progress we have to return cleanly here + // but are still prohibited from doing so since eating the exception would, + // in almost all cases, mess up error handling even more. the only good way + // to handle this is to abort entirely and leave a message, so we'll assert + // (and rethrow anyway, just as a defense against possible NASSERT builds.) + if (std::uncaught_exceptions()) { + assert(false && + "Finally function threw an exception during exception handling. " + "this is not what you want, please use some other methods (like " + "std::promise or async) instead."); + } + throw; + } + } };