How to solve a problem in using RAII code and non-RAII code together in C++?

Question

We have 3 different libraries, each developed by a different developer, and each was (presumably) well designed. But since some of the libraries are using RAII and some don't, and some of the libraries are loaded dynamically, and the others aren't - it doesn't work.

Each of the developers is saying that what he is doing is right, and making a methodology change just for this case (e.g. creating a RAII singleton in B) would solve the problem, but will look just as an ugly patch.

How would you recommend to solve this problem?

Please see the code to understand the problem:

---

My code:

static A* Singleton::GetA()
{
    static A* pA = NULL;
    if (pA == NULL)
    {
        pA = CreateA();
    }
    return pA;
}

Singleton::~Singleton()  // <-- static object's destructor, 
                         // executed at the unloading of My Dll.
{
     if (pA != NULL)
     {
         DestroyA();
         pA = NULL;
     }
}

---

"A" code (in another Dll, linked statically to my Dll) :

A* CreateA()
{
    // Load B Dll library dynamically
    // do all other initializations and return A*
}
void DestroyA()
{
    DestroyB();
}

---

"B" code (in another Dll, loaded dynamically from A) :

static SomeIfc* pSomeIfc;
void DestroyB()
{
    if (pSomeIfc != NULL)
    {
        delete pSomeIfc;  // <-- crashes because the Dll B was unloaded already,
                          // since it was loaded dynamically, so it is unloaded
                          // before the static Dlls are unloaded.
        pSomeIfc = NULL;
    }
}

Answer 1

The simple answer is don't write your own code that dynamically loads/unloads DLL.
Use a framework that handles all the details.

Once a DLL is loaded it is generally a BAD idea to unload it yourself manually.
There are so many gotchas (as you have found).
The simplest solution is to never to unload the DLL (once loaded). Let that OS do that as the application exits.

Answer 2

First, in the example given, I fail to see how Singleton::~Singleton() has access to pA since you declared pA as a static local variable in Singleton::getA().

Then you explained that "B" library is loaded dynamically in A* Create();. Where is it unloaded then? Why doesn't unloading of "B" library happen in void DestroyA() before invoking DestroyB()?

And what do you call "linking a DLL statically"?

Finally, I don't want to be harsh but there are certainly better designs that putting singletons everywhere: in other words, do you really need the object that manages the lifetime of "A" to be a singleton anyway? You could invoke CreateA() at the beginning of your application and rely on atexit to make the DestroyA() call.

EDIT: Since you're relying on the OS to unload "B" library, why don't you remove DestroyB() call from DestroyA() implementation. Then make the DestroyB() call when "B" library unloads using RAII (or even OS specific mechanisms like calling it from DllMain under Windows and marking it with __attribute__((destructor)) under Linux or Mac).

EDIT2: Apparently, from your comments, your program has a lot of singletons. If they are static singletons (known as Meyers singletons), and they depend on each other, then sooner or later, it's going to break. Controlling the order of destruction (hence the singleton lifetime) requires some work, see Alexandrescu's book or the link given by @Martin in the comments.

---

References (a bit related, worth reading)

Clean Code Talks - Global State and Singletons

Once Is Not Enough

Performant Singletons

Modern C++ Design, Implementing Singletons

Answer 3

My answer is the same as every time people go on about singletons: Don't effing do it!

Your singleton causes the destructors to be called too late, after some libraries have been unloaded. If you dropped the "static" and made it a regular object, instantiated in a tighter scope, it'd get destroyed before any libraries got unloaded, and everything should work again.

Just don't use singletons.

Answer 4

You are seeing the side effects of how atexit works in the MS C runtime.

This is the order:

• Call atexit handlers for handlers defined in the executable and static libraries linked with the executable
• Free all handles
• Call atexit handlers for handlers defined in all dlls

So if you load the dll for B manually via a handle this handle will be freed and B will be unavailable in the destructor for A which is in a dll loaded automatically.

I have had similar problems cleaning up critical sections, sockets and db connections in singletons in the past.

The solution is to not use singletons or if you have to, clean them up before main exits. eg

int main(int argc, char * argv [])
{
    A* a = Singleton::GetA();

    // Do stuff;

   Singleton::cleanup();
   return 0;
}

Or even better to use RIIA to clean up after you

int main(int argc, char * argv [])
{
    Singleton::Autoclean singletoncleaner;  // cleans up singleton when it goes out of scope.

    A* a = Singleton::GetA();

    // Do stuff;

   Singleton::cleanup();
   return 0;
}

As a result of the problems with Singletons I try to avoid them completely esp since they make testing hell. If i require access to the same object everywhere and don't want to pass the reference around I construct it as an instance in main, the constructor then registers itself as the instance and you access it like a singleton everywhere else. The only place that has to know it is not a singleton is main.

Class FauxSingleton
{
public:
    FauxSingleton() {
        // Some construction
        if (theInstance == 0) {
            theInstance = this;
        } else {
            // could throw an exception here if it makes sense.
            // I generaly don't as I might use two instances in a unit test
        }
    }

    ~FauxSingleton() {
        if (theInstance == this) {
            theInstance = 0;
        }
    }

    static FauxSingleton * instance() {
        return theInstance;
    }

    static FauxSingleton * theInstance;
};


int main(int argc, char * argv [])
{
    FauxSingleton fauxSingleton;

    // Do stuff;
}

// Somewhere else in the application;
void foo() 
{
   FauxSingleton faux = FauxSingleton::instance();
   // Do stuff with faux;
}

Obviously the constructor and destructor are not thread safe but normally they are called in main before any threads are spawned. This is very useful in CORBA applications where you require an orb for the lifetime of the application and you also require access to it in lots of unrelated places.