From 458fd0c2f79dc51fcdbd29c3ca76b78943dfb0fc Mon Sep 17 00:00:00 2001 From: Nadav Har'El Date: Tue, 30 Jan 2024 21:25:33 +0200 Subject: [PATCH] utils: replace assert() by on_internal_error() In issue #17035 we had a situation where a certain input timestamp could result in the create_time() utility function getting called on a timestamp that cannot be represented as timeuuid, and this resulted in an *assertion failure*, and a crash. I guess we used an assertion because we believed that callers try to avoid calling this function on excessively large timestamps, but evidentally, they didn't tried hard enough and we got a crash. The code in UUID_gen.hh changed a lot over the years and has become very convoluted and it is almost impossible to understand all the code paths that could lead to this assertion failures. So it's better to replace this assertion by a on_internal_error, which by default is just an exception - and also logs the backtrace of the failure. Issue #17035 would have been much less serious if we had an exception instead of an assert. Refs #17035 Refs #7871, Refs #13970 (removes an assert) Signed-off-by: Nadav Har'El --- utils/UUID_gen.hh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/utils/UUID_gen.hh b/utils/UUID_gen.hh index d47c80c7a4..cdb6133472 100644 --- a/utils/UUID_gen.hh +++ b/utils/UUID_gen.hh @@ -17,6 +17,7 @@ #include #include "UUID.hh" +#include "on_internal_error.hh" namespace utils { @@ -383,7 +384,11 @@ public: auto dmc = duration_cast(d); uint64_t msb = dmc.count(); // timeuuid time must fit in 60 bits - assert(!(0xf000000000000000UL & msb)); + if ((0xf000000000000000UL & msb)) { + // We hope callers would try to avoid this case, but they don't + // always do, so assert() would be bad here - and caused #17035. + utils::on_internal_error("timeuuid time must fit in 60 bits"); + } return ((0x00000000ffffffffL & msb) << 32 | (0x0000ffff00000000UL & msb) >> 16 | (0x0fff000000000000UL & msb) >> 48 |