0

I'd start learning C++ and I don't understand this is memory leak or some kind of voodoo magic?!

I have some "singleton" class (just for a demo):

#include <iostream>
using namespace std;

class S {
    private: S() {
        cout << "S::S" << endl;
    }

    public: static S* instance() {
        static S* i;
        if(i == NULL) {
            cout << "Create S" << endl;
            i = new S;
        }
        return i;
    }

    public: virtual ~S() {
        cout << "S::~S" << endl;
    }

    public: void a() {
        cout << "S::a" << endl;
    }
};

int main() {
    // call some method
    S::instance()->a();
    // delete pointer to instance (no reason, because I'm curious)
    delete S::instance();
    // call method again
    S::instance()->a();

    return 0;
}

Output of these is:

Create S
S::S
S::a
S::~S
S::a

So my question is: Why after destructor call I still have working copy of class S in static variable?

Upd: Thanks for answers. I realize my mistakes. Please, forgive me for disturbing.

Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
lxmarduk
  • 23
  • 2
  • 3
    You don't have a working copy, you have a *dangling* pointer and are really triggering undefined behaviour.. (btw - you don't need `public:` before *each method* this is not java.. – Nim Oct 14 '14 at 09:46
  • 1
    (btw - to the down voters - it's a bit harsh to down vote a beginner - specially without any remarks as to why!) – Nim Oct 14 '14 at 09:48
  • 1
    `I don't understand this is memory leak or some kind of voodoo magic` The bottom line is this - unlike most other languages, in C++, doing "bad things" doesn't mean the code will crash, get a stack trace, pop up a message box saying "you did a bad thing", etc. So the answers that you're getting not only apply to your question, but any "nutty" C++ code that happens to compile without error, but seems to run ok. – PaulMcKenzie Oct 14 '14 at 10:07
  • Relevant: http://stackoverflow.com/questions/5431420/why-doesnt-the-program-crash-when-i-call-a-member-function-through-a-null-point – M.M Nov 05 '14 at 10:38

4 Answers4

2

The second call to instance is now risky:

S::instance()->a();

It will call:

public: static S* instance() {
    static S* i;
    if(i == NULL) {
            cout << "Create S" << endl;
        i = new S;
    }
    return i;
}

But i is not NULL (although to object is deleted, the pointer to it is invalid), so it returns an pointer to some memory where once was S. And then you have undefined behaviour, the old object maybe there, it may not, or something far worse can happen.

Conclusion to make it work, make sure when you delete the object always set the pointer to NULL. For example check this answer: How to delete Singleton pointer?

Community
  • 1
  • 1
RvdK
  • 19,580
  • 4
  • 64
  • 107
  • I like your answer better. You caught on to something I missed and is probably what the OP actually wanted to know: why instance() didn't cause a reconstruction. – Edward Strange Oct 14 '14 at 09:55
  • Ok. I get it (I'll never use such code in real life). But why it doesn't call destructor at end of the program (I think it should)?! – lxmarduk Oct 14 '14 at 10:41
  • after 'S::a' you mean? Because it's a pointer, you should clear it yourself. Your OS or IDE just frees the memory used by the program, no need to call a destructor. – RvdK Oct 14 '14 at 10:45
0

You don't have a "working instance," you have Undefined Behaviour (you're accessing an object after it's been destroyed). It just so happens that because a() does not access the data members of the object in any way, it appears to work normally. But you're technically dereferencing a dangling pointer and anything could happen, including segfaults.

While your case is slightly different (you have an access-after-delete, not a dangling pointer to stack), you might want to read this excellent answer on stale pointers.


Side note on formatting: while your code layout is certainly legal, it reads quite foreign to a C++ developer. Here's what more natural formatting could look like:

class S {
    S() {
        cout << "S::S" << endl;
    }

public:
    static S* instance() {
        static S* i;
        if(i == NULL) {
            cout << "Create S" << endl;
            i = new S;
        }
        return i;
    }

    virtual ~S() {
        cout << "S::~S" << endl;
    }

    void a() {
        cout << "S::a" << endl;
    }
};
Community
  • 1
  • 1
Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
0

Because using a destroyed object results in undefined behavior. Anything can happen, including it working...until it decides not to.

That's the correct answer by the language. The reason this usually results in this kind of undefined behavior is because member functions are just regular functions once turned into machine code. If you never actually use a member of your class it's unlikely to explode.

But again, undefined behavior. Don't do this expecting it's going to work.

Edward Strange
  • 40,307
  • 7
  • 73
  • 125
0

As in other answers, you have undefined behaviour in your code, in simple code like your you might see that your code works actually properly anyway. In most codebases, code is much more complicated, and such bug results in :

bash: line 7: 7324 Segmentation fault (core dumped) ./a.out

But still this might occur in other parts of code, not related to the one where bug is present, and also not immediately.

I have modified your code a bit, to allow for such output. As you see there is more allocations in this code, and this is what most probably is causing this segfault, by that I mean that vector is probably reusing part of S::ss freed memory causing it to crash dump once used in S::a.

http://coliru.stacked-crooked.com/a/c1be7a83275ae847

#include <iostream>
#include <vector>
#include <string>
using namespace std;

class S {
    std::string ss;
    private: S() {
        ss = "ub ub ub ub ub ub ub ub ";
        cout << "S::S" << endl;
    }

    public: static S* instance() {
        static S* i;
        if(i == NULL) {
            cout << "Create S" << endl;
            i = new S;
        }
        return i;
    }

    public: virtual ~S() {
        cout << "S::~S" << endl;
    }

    public: void a() {
        cout << "S::a" << ss << endl;
    }
};

int main() {
        S::instance()->a();
        // delete pointer to instance (no reason, because I'm curious)
        delete S::instance();    
    for ( int n = 0; n < 100; ++n ){
        std::vector<char> avec(n);
        // call some method
        S::instance()->a();
    }

    return 0;
}
marcinj
  • 48,511
  • 9
  • 79
  • 100