How horrible is Static-Casting Base class to Derived class in order to call destructor in tests (for tests)?

Question

Preface: I am aware that the following is a very dirty hack. We will refactor things (so we can write non-hacky tests, use Mocks and possibly even get rid of the Singletons), but first we need test coverage...

As said before, we have a significant number of "Singleton" type classes in our project, i.e. something along the lines of.

class SomeSingleton
{
  public:
    static SomeSingleton& getSingleton()
    { 
        if (m_singletonInstance == nullptr)
            m_singletonInstance = new SomeSingleton();
        return *m_singletonInstance;
    }

  protected:
    SomeSingleton();
    virtual ~SomeSingleton();

    static SomeSingleton* m_singletonInstance = nullptr;
}

In order to properly run our unit tests the Singleton needs to be "reset" (or data from our previous tests may persist in the singleton, influencing the current test). Unfortunately there is no feasible way to do that (the Singleton instance and the destructor are protected).

We do not wish to change our production code to contain test-only functionality (i.e. no forward declaring a ::test::SomeSingletonResetter as a friend in SomeSingleton, no introducting a public void resetSingleton_forTestUseOnly() function etc.) - and we also wish to avoid any significant changes to the production code until we have test coverage in place.

So, here's the dirty hack we are considering: In the test project we derive a very simple wrapper class (i.e. no members) from the Singleton with a static function that deletes the singleton - however as the destructor cannot be called even from the derived class (it is protected) we would static_cast the singleton to the derived class and delete that:

class SomeSingletonWrapper : public SomeSingleton
{
  public:
    SomeSingletonWrapper();
    ~SomeSingletonWrapper() override;

    static void reset()
    {
        //can't delete/call destructor on SomeSingleton since it is protected
        delete static_cast<SomeSingletonWrapper*>(m_singletonInstance);
        m_singletonInstance = nullptr;
    }
}

Our thoughts are, the classes are essentially the same size, and the derive classes' destructor will call the base destructor, so the class gets destroyed properly. Can this work (until we can refactor things) or will it horribly explode in our faces? And is there a better (less hacky) way to archive this (without majorly modifying the production code)?

Edit:

I am aware that the alternative would have been to simply not call the destructor (and only set m_singletonInstance = nullptr) but that would be even worse since I now leak memory with every single test and those singletons keep floating around until the test app terminates, doing god-knows-what... brrr....

Answer 1

According to the standard 5.3.5.2:

In the first alternative (delete object), the value of the operand of delete may be a null pointer value, a pointer to a non-array object created by a previous new-expression, or a pointer to a subobject (1.8) representing a base class of such an object (Clause 10). If not, the behavior is undefined.

Since you are deleting a super-class, not a sub-class, and the pointer was not previously created as an instance of that super-class, you are in undefined territory. So you may get lucky and it will work sometimes on some systems with some compiler, but there is no guarantee.

Can you override getSingleton()? It's not virtual or static in the code given, but it should probably be one of those. If the former, there's an easier solution (override it in your wrapper). If not, you might consider trying to replace the created value with your wrapper:

class SomeSingletonWrapper : public SomeSingleton
{
public:
    static void injectWrapper()
    {
        // Should be null or else someone used it before we replaced it
        if( m_singletonInstance != nullptr ) 
        {
            throw std::exception( "Singleton wrapping failed!" );
        }
        m_singletonInstance = new SomeSingletonWrapper();
    }

    static void resetWrapper()
    {
        auto wrapper = dynamic_cast<SomeSingletonWrapper*>( m_singletonInstance );
        if( wrapper == nullptr )
        {
            throw std::exception( "Singleton wrapping failed in the reset!" );
        }

        delete wrapper;
        m_singletonInstance = nullptr;
    }
}

Then in your test, replace it before anyone uses it (but beware the static order initialization fiasco where something else might get the old instance first):

class MyTest
{
public:
    void testSetup()
    {
        SomeSingletonWrapper::injectWrapper();    
    }

    void testCleanup()
    {
        SomeSingletonWrapper::resetWrapper();
    }

    void testMySingletonUse()
    {
        auto& single = SomeSingleton::getInstance();
        ASSERT( dynamic_cast<SomeSingletonWrapper*>( &single ) ); // Yay! It worked!
        // More testy stuff using 'single'
    }
}