icinga2
icinga2 copied to clipboard
Icinga
Fix a crash when the worker threads log on shutdown
This fixes the problem described in the referenced issue, as it now just hold a lock on itself instead
of static mutex when logging to a console. It's the case that when starting a process, e.g. to validate
Icinga2 configs, and calling exit(3) after validation succeeds, then the static ProcessEntry() method
from StreamLogger class is used to process the entries. See Logger::~Logger() for details. However, the
problem here is that after the process has entered to exit(3), work queue threads still log something, and
of course a static mutex from the stream logger class is used to ensure std::cout thread-safety. But this
static mutex may already have been destroyed. And since we don't have a handler for this use
case (to lock a dead mutex), it will directly call std::terminate(), as it immediately raises a system error.
Our unhandled exception handler of course tries to generate statcktrace from this situation but, for some reason
(could be due to the same reasone as above) the static variables in exception.cpp such as l_lastExceptionObj
etc.. aren't set, which in turn causes a segfault in RethrowUncaughtException().
fixes #9249
So if I understand correctly, the main change that prevents that crash is that the mutex is moved to a ref-counted object that is kept alive for longer using this reference here:
https://github.com/Icinga/icinga2/blob/94bd28e6ba0401e073856b19859f6a84c939bc06/icinga-app/icinga.cpp#L298
So if Main() (yes, upper-case "M") returns, it's freed and if some WorkQueue was still alive at that point, that crash might still appear I guess.
While I'm not a huge fan of the workaround in #9497, that seems to be the more pragmatic approach unless we'd want to go down the "analyze all ctor/dtor ordering" rabbit hole.
So if
Main()(yes, upper-case "M") returns, it's freed and if someWorkQueuewas still alive at that point, that crash might still appear I guess.
But we could also initialize this instance at the very beginning and put it to the global namespaces! Just thinking out loud! 🤷