-2

I have the following singletone implementation:

class B   
{
  public:
    static B *getInstance()
    {
        if ( !m_data )
          m_data = new(std::nothrow) B;
        return m_data;
    }

  private:
    static B *m_data;

  private:
    B() { std::cout << "B::B() " << B::m_data << std::endl; }
   ~B() { std::cout << "B::~B()" << std::endl; }
    B( const B & ) = delete;
    B &operator=( const B & ) = delete;
};


B *B::m_data = nullptr;

In main() I have:

B *pSingletone = B::getInstance();

I was surprised to see that destructor was never called after I killed my program. What am I missing? Do we need destructor at all in this case? My questions are about destructor only, not how bad or good singletons are. I know I don't have to allocate it on the heap.

Edit to summarize: as several people pointed out, only static pointer is destructed on program termination, not the object it pointed to. To destruct the object, explicit "delete" is needed. After the program terminated, the allocated memory is of course returned to the system but without destructor call. Thanks everybody!

TT_ stands with Russia
  • 1,785
  • 3
  • 21
  • 33
  • 1
    https://stackoverflow.com/questions/1008019/c-singleton-design-pattern?rq=1 – TT_ stands with Russia Jan 31 '18 at 00:35
  • 6
    How did you detect that the destructor of the *pointer* `m_data` wasn't called? I'm pretty sure it *was* called but had no effect... – Dietmar Kühl Jan 31 '18 at 00:37
  • Yes it's compiled. I think it's more or less usual singleton implementation (with allocation on the heap). – TT_ stands with Russia Jan 31 '18 at 00:38
  • 2
    The destructor of `m_data` will be called, but it's just a dumb pointer that doesn't do anything. The question is how `*B::getInstance()` is stored. – nwp Jan 31 '18 at 00:39
  • If you do manual memory management you have to go all the way and `delete B::getInstance();`. Alternatively use `std::unique_ptr` and `std::make_unique` so you don't need to worry about these things. – nwp Jan 31 '18 at 00:40
  • Just a note on the general approach: singletons are a bad idea and they become worse over time [as concurrency increases and singletons don't mix with concurrency at all]. If you insist to create a `static` object, at least make the initialization thread-safe, i.e., move the object *into* the function and initialize it in the definition, e.g. `B* foo(){ static B* rc = new B(); return rc; }`. – Dietmar Kühl Jan 31 '18 at 00:41
  • 2
    The destructor of the pointer was called, but not the destructor of the thing it points to. – Galik Jan 31 '18 at 00:43
  • @amin It's said "Destructors (12.4) for initialized objects of static storage duration (declared at block scope or at namespace scope) are called as a result of returning from main and as a result of calling exit". But I killed my program - should not it be called and print the message? – TT_ stands with Russia Jan 31 '18 at 00:43
  • 3
    The object of `class B` is not static, it is created dynamically on the heap. Only the pointer pointing to it is static. – Galik Jan 31 '18 at 00:44
  • 3
    TT_ note that in the question you cite in your top comment only the question uses that style of singleton. The overwhelmingly upvoted answer recommends that you NOT use that style of singleton. There are a number of reasons, not the least being the problem you've just run into. – user4581301 Jan 31 '18 at 01:16

2 Answers2

4

The destructor isn't being called because you're calling new without ever calling delete on m_data. I propose the following to avoid manual allocation, getting rid of m_data:

static B *getInstance(){
    static B theInstance; // initialized during first call, destroyed after main exits
    return &theInstance;
}
alter_igel
  • 6,899
  • 3
  • 21
  • 40
0

Why would the destructor get called ? You're not deleting the object. In other words, you have a memory leak.

You could fix this by including this in the end of main():

delete pSingletone;

You would have to make B::~B() public, though.

Sid S
  • 6,037
  • 2
  • 18
  • 24