From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Wed, 16 Aug 2023 17:53:41 +0000 (+0000)
Subject: Cover OnTerminate() calls unrelated to exception handling (#1430)
X-Git-Tag: SQUID_7_0_1~368
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=cf2fca525520630bfdb423793ccbbbfba5eba680;p=thirdparty%2Fsquid.git

Cover OnTerminate() calls unrelated to exception handling (#1430)

The C++ standard lists many reasons[^1] for calling std::terminate().
All of them deal with an "exception handling failure". However, when
Squid bugs lead to an undefined behavior (e.g., by calling a pure
virtual function), some compilers also call std::terminate(). In those
cases, there may be no active exception, and our std::terminate()
handler's report about an "exception handling failure" (with "no active
exception") was confusing and misleading.

Also do not describe an exception when there is none. Callers that know
that the exception exists may continue to use CurrentException(),
especially if they do not need to report that exception on a dedicated
debugs() line. Other callers, including OnTerminate(), should use the
new CurrentExceptionExtra() manipulator.

[^1]: https://en.cppreference.com/w/cpp/error/terminate
---

diff --git a/src/base/TextException.cc b/src/base/TextException.cc
index 3c34245f9b..7f61471d5a 100644
--- a/src/base/TextException.cc
+++ b/src/base/TextException.cc
@@ -65,25 +65,41 @@ operator <<(std::ostream &os, const TextException &ex)
     return os;
 }
 
+/// prints the current exception (which presence has been verified by the caller)
+static std::ostream &
+CurrentException_(std::ostream &os)
+{
+    try {
+        throw; // re-throw to recognize the exception type
+    }
+    catch (const TextException &ex) {
+        os << ex; // optimization: this is a lot cheaper than what() below
+    }
+    catch (const std::exception &ex) {
+        os << ex.what();
+    }
+    catch (...) {
+        os << "[unknown exception type]";
+    }
+    return os;
+}
+
 std::ostream &
 CurrentException(std::ostream &os)
 {
     if (std::current_exception()) {
-        try {
-            throw; // re-throw to recognize the exception type
-        }
-        catch (const TextException &ex) {
-            os << ex; // optimization: this is a lot cheaper than what() below
-        }
-        catch (const std::exception &ex) {
-            os << ex.what();
-        }
-        catch (...) {
-            os << "[unknown exception type]";
-        }
+        os << CurrentException_;
     } else {
         os << "[no active exception]";
     }
     return os;
 }
 
+std::ostream &
+CurrentExceptionExtra(std::ostream &os)
+{
+    if (std::current_exception())
+        os << Debug::Extra << "exception: " << CurrentException_;
+    return os;
+}
+
diff --git a/src/base/TextException.h b/src/base/TextException.h
index e04ee92939..a2c9c317e2 100644
--- a/src/base/TextException.h
+++ b/src/base/TextException.h
@@ -52,6 +52,10 @@ public:
 /// prints active (i.e., thrown but not yet handled) exception
 std::ostream &CurrentException(std::ostream &);
 
+/// If there is an active (i.e., thrown but not yet handled) exception, reports
+/// it on a dedicated DebugExtra line. Otherwise, does nothing.
+std::ostream &CurrentExceptionExtra(std::ostream &);
+
 /// efficiently prints TextException
 std::ostream &operator <<(std::ostream &, const TextException &);
 
diff --git a/src/main.cc b/src/main.cc
index 4c19f7e4ab..41cfb194d8 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -1305,7 +1305,7 @@ OnTerminate()
         return;
     terminating = true;
 
-    debugs(1, DBG_CRITICAL, "FATAL: Dying from an exception handling failure; exception: " << CurrentException);
+    debugs(1, DBG_CRITICAL, "FATAL: Dying after an undetermined failure" << CurrentExceptionExtra);
 
     Debug::PrepareToDie();
     abort();