2

I have a class that provides a utility for logging errors and exceptions. This is part of a library that's integrated into other applications. This class is implemented via the Singleton pattern (Meyer's singleton to be specific). The issue arises during the destruction phase when there's no defined point in time when this class is destructed. It so happens that during destruction an error/exception is raised which tries to access this class and leads to undefined behavior. Any flag within the class to maintain its state seemed moot since accessing the flag itself is UB once the object is destructed. I can only think of having the object allocated on the heap and letting it linger till the program execution but an issue with that is instruments designed to detect memory leaks would report this. I've also tried installing an std::atexit() handler but that too was invoked before the destruction of the object which doesn't help. How are such issues of lifetime best handled?

EDIT: Here's a short snippet to illustrate the problem

#include <iostream>
#include <memory>

#define LOG_EXCEPTION(msg) Logger::instance()->logException(msg)
#define LOG_MESSAGE(msg) Logger::instance()->logMessage(msg)

struct Logger {
    Logger() = default;
    ~Logger() {
        std::cout<<"~Logger\n";
    }
    int x;
    public:
    static const std::shared_ptr<Logger>& instance() {
        static auto logger = std::make_shared<Logger>();
        return logger;
    }
    void logException(std::string const& msg) {
        ++x;
        std::cout<<msg<<'\n';
    }
    void logMessage(std::string const& msg) {
        ++x;
        std::cout<<msg<<'\n';
    }
};

struct Foo {
    ~Foo() {
        //Something wrong, so log an exception
        LOG_EXCEPTION("oops");
    }
};

int main()
{
    static Foo f;
    LOG_MESSAGE("message"); // Just to force creation of the Logger object
}

As is evident, since Foo f outlives the singleton instance, it is UB to access the Logger instance from its destructor. This is just a demonstrative code but such logging of exceptions can happen at any point in the program sequence and it's difficult to reason about the ordering of the statics (the actual error isn't even from a static instance).

Zoso
  • 3,273
  • 1
  • 16
  • 27
  • Please provide a [mre]. Also, if you think your initial design decision is causing so much problems, maybe you should rethink them. – kiner_shah Feb 16 '23 at 07:39
  • 4
    Forget about using singletons, use dependency injection. Give your class an abstract baseclass (interface) and inject that to the classes that need it. Then you can create your logger in one of the first lines of main and pass it around to those classes that needed it (they only need to know the interface). Added bonus during unit testing you can inject an implementation of the log interface that does nothing (and you really only test the functionality of the class that will also log). – Pepijn Kramer Feb 16 '23 at 07:41
  • 1
    There is another option : Each class that logs, logs to an interface with methods it specifies. Then you can implement that interface and that implementation can forward to the log infrastructure you need (complete decoupling from a standard logging interface). Anyway in bigger systems ... stay away from singletons. – Pepijn Kramer Feb 16 '23 at 07:43
  • The last solution allows you to build a static lib with only your class (and the interface it wants to log too). This makes it extremely unit testable. You can even verify log calls will be made at the correct time (google_mock). Such an interface would have methods like `virtual void report_file_could_not_be_found(std::string filename) = 0`. Which is totally independent of your logging (today it is files, tomorrow it might be an elastics server) – Pepijn Kramer Feb 16 '23 at 08:00
  • That's the main flaw of that type of singleton. It gets exposed in cases of a) several singletons depending on each other, b) destructors of objects with process-wide objects relies on the singleton. Possible solutions are either to avoid those scenarios or design against them 1) do not use multiple singletons 2) do not use Meyer's but a resurrectable singleton ("phoenix") 3) implement some kind of interface that allows to skip steps relying on singleton's existence. 4) do NOT use singletons at all. – Swift - Friday Pie Feb 16 '23 at 08:04
  • @PepijnKramer Maybe a bit too strict – singletons are destructed [in reverse order](https://stackoverflow.com/a/246568/1312382) to their construction – so if we avoid global variables entirely and provide *all* global data as singletons we only need to assure calling the instance functions in correct order within `main` right before starting any further threads and we should be safe. From a practical point of view I'd still rather advocate against singletons, though. – Aconcagua Feb 16 '23 at 08:05
  • @Aconcagua the problem is that the moment of that destruction itself is not defined and sadly.... in real cases the order of destruction is implementation defined if those variables were in separate compilation units. – Swift - Friday Pie Feb 16 '23 at 08:08
  • @Aconcagua But with (dynamic) libraries that order can still be hard to get right. So I'd rather be strict. Because I've seen to many issues with this. And then there is still the issue of unit testability (in bigger projects) – Pepijn Kramer Feb 16 '23 at 08:08
  • Actually I have such a design in a specific case – exactly one singleton for logging purposes (allows a lean/convenient interface), no global objects at all apart from one single pointer. The pointer is assigned an object constructed and destructed from within `main` and this object internally uses dependency injection for all further objects other than the logger (but that's less convenient). Sole reason for the pointer is to be able to access a specific property from the windows service control routine to assure correct service shutdown (or on planned port to linux for the signal handler)... – Aconcagua Feb 16 '23 at 08:15
  • Not to get me wrong: I do not want to advocate for singletons in the first place. Just that I accept the use of the word *'never'* just in one single sencence: *'Never say "never"'*. There always are scenarios where potentially problematic patterns can successfully be applied – **if** one knows how to do so correctly. Admitted, if applied in libraries it can make correct usage of pretty hard in projects using these libraries, so at least less recommendable there... – Aconcagua Feb 16 '23 at 08:27

1 Answers1

0

In an overly simplified way that would depend on your code not having other singletons or globals that use in their destructors this Logger you could just:

int main()
{
    LOG_MESSAGE("start message"); // Just to force creation of the Logger object
    {
        static Foo f;
        //All the rest of your code here.
    }//Everything else is destroyed here.
     //This could be a function instead of a block.
}//Logger is destroyed here

But if the preconditions are not met (other globals/singletons) then moving away from the Meyers Singleton will be necessary. A "reviving" singleton or dependency injection as suggested in the comments would be good possibilities.

Ignacio Gaviglio
  • 56
  • 5