Design: How to avoid calling destructor?

Question

I got a Window class that is a wrapper to some C struct.
The class has a static vector<Window*> windows_ that is a list containing created windows.
Window constructor does two things:

• handle_ = SDL_CreateWindow( ... ); that basically allocates the C struct and store the pointer in a member variable handle_;
• pushes this in the list.

Window destructor does three things but only if handle_ is not a nullptr:

• SDL_DestroyWindow() deallocates the C struct;
• removes this from the list.
• handle_ = nullptr;

Then, in my main I declare a Window as a local variable.
When the window receives the CLOSE event, I call that window's destructor.
Then, when the window goes out of scope, the window's destructor gets called again and I receive a segmentation error.

I know expicitly calling a destructor is delicate but I don't really know why.
So the question is twofold:

Why is it crashing?

What design can I use to avoid calling the destructor?

Answer 1

You should post some code so we can see exactly what is happening, but you're right that you shouldn't manually call the destructor because that will cause undefined behavior (see AliciaBytes' answer). Instead, add a method to the class to close the window, and use the class to provide a safe interface to the SDL window functions. Here is a sketch:

class Window {
private:
    SDL_Window *window_;

public:
    Window() : window_(nullptr) { }
    Window(const Window &) = delete;
    Window(Window &&w) : window_(w.window_) { w.window_ = nullptr; }
    Window(const char* title, int x, int y, int w, int h, Uint32 flags)
        : window(SDL_CreateWindow(title, x, y, w, h, flags))
    { }

    ~Window()
    {
        if (window_ != nullptr)
            SDL_DestroyWindow(window_);
    }

    Window &operator=(const Window &) = delete;
    Window &operator=(Window &&w)
    {
        if (window_) { SDL_DestroyWindow(window_); window_ = nullptr; }
        window_ = w.window_;
        w.window_ = nullptr;
    }

    operator bool() const
    {
        return window_ != nullptr;
    }

    void close()
    {
        if (window_ != nullptr) {
            SDL_DestroyWindow(window_);
            window_ = nullptr;
        }
    }
};

Answer 2

Calling the destructor is delicate because it leaves an object in an undefined state. Any read from this object is undefined. In praticular, calling the destructor second time - which will always happen for local variables.

I think in your case you mixed the operations on the list with those on the window.

A constructor should only stabilize the invariant on the current object. Putting it inside a list should not be the responsibility of the constructor, in normal circumstances.

Don't call the destructor on CLOSE. Deallocate the struct, and put nullptr in _handler. That's all.

Answer 3

Your code needs to keep state as to whether your resource was cleaned up. Typically you set a handle to -1 or a pointer to nullptr:

class MyResourceContainer {
    MyResourceContainer(int someParameter) {
        open(someParameter);
    }

    ~MyResourceContainer() {
        close();
    }

    void open(int someParameter) {
        if(handle > -1) {
            // throw exception or close(), whatever you prefer
        } 

        // allocate resource using specified parameter
        handle = 42;
    }

    // can be called multiple times
    void close() {
        if(handle > -1) {
            // release resource
            handle = -1;
        }
    }

private:
    int handle = -1;
};

Answer 4

You really shouldn't be calling the destructor to variables with automatic storage explicitly, this is undefined behavior:

Standard 12.4[class.dtor]/15

Once a destructor is invoked for an object, the object no longer exists; the behavior is undefined if the destructor is invoked for an object whose lifetime has ended (3.8). [ Example: if the destructor for an automatic object is explicitly invoked, and the block is subsequently left in a manner that would ordinarily invoke implicit destruction of the object, the behavior is undefined. —end example ]

instead of looking for a "hacky" way of disabling the automatic destructor call or something else you should probably look for a better design.

Making a DestroyWindow() member function that takes care of the cleanup and sets a flag inside the class, but having the destructor check that flag and destroy the window (+ the other cleanup) if it wasn't done manually would already fix a lot.

---

To make it clear you probably want it like the fstream and other streams in the standard library. You can explicitly call close on them, but if you don't do it it gets done in the destructor for you as to not leak resources. But note that this shouldn't be done through explicit destructor calls. (fstream for example got a close member function for explicitly closing it.)