Search code examples
c++design-patternssingletondestructorself-destruction

Why does the destructor call itself endlessly (causing a stack overflow)?


I am confused on why the destructor calls itself an endless amount of times, when I try to construct an object say LeakySingleton on the heap through a static function call create_instance() and then try to delete it explicitly afterwards via the delete operation.

As I understand it, considering the source listings from below, the variable leaky_singleton inside main() gets to point to the heap allocated resource returned by create_instance(). Thus, we have allocated an object LeakySingleton on the heap indirectly through the create_instance function. Now, if I explicitly call the delete operator or delete function on leaky_singleton, then it calls the destructor first and checks whether or not it satisfies the instance != nullptr condition and then the deletion of the object that instance points to should be deleted. If this object LeakySingleton::instance is deleted, then there is no reason for the dtor to call itself again, or am I missing something here?

Calling it with and without valgrind leads to a segmentation fault (invalid memory access due to stack overflow):

Segmentation fault (core dumped)

Stepping through with the debugger, leads to endless destructor calls (the culprit of the stack overflow).

From cplusplus.com (http://www.cplusplus.com/forum/general/40044/):

If you delete your object, it attempts to delete itself, which will cause it to attempt to delete itself, which will cause it to delete itself, which will...

Why does it attempt to delete itself, when I simply use the delete operator/function to deallocate the heap object LeakySingleton pointed to by the static class member variable LeakySingleton::instance? The heap allocated resource is pointed to by the LeakySingleton::instance pointer variable which points to a LeakySingleton object. So why does an explicit delete function call doesn't delete or rather deallocate the allocated heap object and instead recurses endlessly? What am I missing here?

(My current dtor and ctor understanding: The new function/operator allocates memory for an object on the heap and calls the constructor, and the delete function calls the destructor and in my case also calls a delete operator/function inside.)

Source:

main.cpp:

class Singleton final
{
    public:
        static Singleton & create_instance(int);
        ~Singleton() = default;
    private:
        int x;
        Singleton(int);

        Singleton(Singleton &) = delete;
        Singleton(Singleton &&) = delete;
        Singleton & operator=(Singleton &) = delete;
        Singleton & operator=(Singleton &&) = delete;
};

Singleton::Singleton(int t_x) : x{t_x}
{}

Singleton & Singleton::create_instance(int t_x)
{
    static Singleton instance{t_x};
    return instance;
}

// Potential endless dtor calls inside:
class LeakySingleton final
{
    public:
        static LeakySingleton * create_instance(int);
        ~LeakySingleton();
    private:
        int x;
        static LeakySingleton * instance;
        LeakySingleton(int);

        LeakySingleton(LeakySingleton &) = delete;
        LeakySingleton(LeakySingleton &&) = delete;
        LeakySingleton & operator=(LeakySingleton &) = delete;
        LeakySingleton & operator=(LeakySingleton &&) = delete;
};

LeakySingleton * LeakySingleton::instance = nullptr;

LeakySingleton::LeakySingleton(int t_x) : x{t_x}
{}

LeakySingleton::~LeakySingleton()
{
    if (instance != nullptr)
    {
        delete instance;
        instance = nullptr;
    }
}

LeakySingleton * LeakySingleton::create_instance(int t_x)
{
    if (instance == nullptr)
    {
        instance = new LeakySingleton{t_x};
    }
    return instance;
}

int main()
{ 
    // The correct implementation with no issues:
    {
        Singleton & singleton = Singleton::create_instance(42);
    }

    // The faulty implementation causing the dtor to recurse endlessly and resulting in a segfault:
    {
        LeakySingleton * leaky_singleton = LeakySingleton::create_instance(42);
        delete leaky_singleton;
    }

    return 0;
}

Makefile:

CC = g++
CFLAGS = -g -Wall -Wextra -pedantic -std=c++11
SRC = main.cpp
TARGET = app
RM = rm -rf

.PHONY: all clean

all: $(TARGET)

clean:
    $(RM) $(TARGET)

$(TARGET): $(SRC)
    $(CC) $(CFLAGS) $^ -o $@

Solution

  • In C++, delete will call the class destructor.

    The delete statement in your main function is calling LeakySingleton::~LeakySingleton, which in turn tries to delete the static instance pointer, which then calls the destructor again. Your code never had the chance to set the static pointer to null. There you have an infinite loop.

    P.S. IMHO, it is generally a bad practice to modify static members in non-static methods. I believe you can put the static clean up logic in another static method.

    class LeakySingleton final {
    public:
      static LeakySingleton& create_instance(int);
      static void destroy_instance();
      ~LeakySinglton() = default;
    private:
      static LeakySingleton *instance;
      ...
    };
    
    void LeakySingleton::destroy_instance() {
      if (instance != nullptr) {
        delete instance;
        instance = nullptr;
      }
    }