CODEWITHSUNDEEP
c++destructormultiple-inheritancethis-pointer
AnorZaken
AnorZaken

Reputation: 2124

Why the this-pointer address is something else than expected in the destructor (c++)

I have a weird problem with a this-pointer in a base-class destructor.

Problem description:

I have 3 classes: A1, A2, A3

A2 inherits publicly from A1 and inherits privately from A3

class A2:private A3, public A1 {...}

A3 has a function getPrimaryInstance() ...that returns an A1-type reference to an A2 instance:

A1& A3::getPrimaryInstance()const{
    static A2 primary;
    return primary;
}

And the A3 constructor looks like this:

A3(){
    getPrimaryInstance().regInst(this);
}

(Where regInst(...) is a function defined in A1 that stores pointers to all A3 instances)

Similarly the A3 destructor:

~A3(){
    getPrimaryInstance().unregInst(this);
}

^Here is where the problem occurs!

When the static A2-instance named primary gets destroyed at program termination the A3-destructor will be called, but inside ~A3 I try to access a function on the same instance that I a destroying. => Access violation at runtime!

So I thought it would be possible to fix with a simple if-statement like so:

~A3(){
    if(this != (A3*)(A2*)&getPrimaryInstance()) //Original verison
    //if (this != dynamic_cast<A3*>(static_cast<A2*>(&getPrimaryInstance())))
    //Not working. Problem with seeing definitions, see comments below this post

        getPrimaryInstance().unregInst(this);
}

(Reason for double cast is the inheritance:)
A1 A3
. \ /
. A2
(But it's not important, could have just (int)-casted or whatever)

The kicker is that it still crashes. Stepping through the code with the debugger reveals that when my A2 primary-instance gets destroyd the this-pointer in the destructor and the address I get from calling getPrimaryInstance() doesn't match at all for some reason! I can't understand why the address that the this-pointer points to is always different from what it (to my limited knowledge) should be. :(

Doing this in the destructor:

int test = (int)this - (int)&getPrimaryInstance();

Also showed me that the difference is not constant (I briefly had a theory that there be some constant offset) so it's like it's two completely different objects when it should be the same one. :(

I'm coding in VC++ Express (2008). And after googling a little I found the following MS-article:
FIX: The "this" Pointer Is Incorrect in Destructor of Base Class

It's not the same problem as I have (and it was also supposedly fixed way back in C++.Net 2003). But regardless the symptoms seemed much alike and they did present a simple workaround so I decided to try it out:
Removed the not-working-if-statement and added in virtual in front of second inheritance to A2, like so:

class A2:private A3, public A1 {...} // <-- old version
class A2:private A3, virtual public A1 {...} //new, with virtual!

AND IT WORKED! The this-pointer is still seemingly wrong but no longer gives an access-violation.

So my big question is why?
Why does the this-pointer not point to the where it should(?)?
Why does adding virtual to the inheritance like above solve it (despite this still pointing somewhere else than &getPrimaryInstance())?
Is this a bug? Can someone try it in a non-MS environment?
And most importantly: Is this safe?? Sure it doesn't complain anymore, but I'm still worried that it's not doing what it should. :S

If anyone has knowledge of or experience from this and could help me understand it I would be very thankful since I'm still learning C++ and this is entirely outside of my current knowledge.

Upvotes: 7

Views: 1953

Answers (8)

aschepler
aschepler

Reputation: 72401

OP @AnorZaken commented:

...This was one of the original problems I tried to tackle: I would like getPrimaryInstance() to return an A2 reference directly, but I can't! A3 hasn't seen the definition of A2! Since getPrimaryInstance() is declared in the base-class of A3 (not mentioned above) you get: error C2555: 'A3::getPrimaryInstance': overriding virtual function return type differs and is not covariant from 'A3Base::getPrimaryInstance' Simply: even if I declare the existence of A2 I don't know of any way to tell the compiler that A2 has A1 as base before I declare A2. :( If I could solve this it would be the great!

So it sounds like you have something like:

class A3Base {
public:
  virtual A1& getPrimaryInstance();
};

And since class A2 cannot be defined before class A3, I would just skip the covariant return type. If you need a way to get the A2& reference from an A3, add that as a different method.

// A3.hpp
class A2;

class A3 : public A3Base {
public:
  virtual A1& getPrimaryInstance();
  A2& getPrimaryInstanceAsA2();
};

// A3.cpp
#include "A3.hpp"
#include "A2.hpp"

A1& A3::getPrimaryInstance() {
    return getPrimaryInstanceAsA2(); // no cast needed for "upward" public conversion
}

Upvotes: 1

Loki Astari
Loki Astari

Reputation: 264501

You use of C casts is killing you.
It is especially liable to break in situations with multiple inheritance.

You need to use dynamic_cast<> to cast down a class hierarchy. Though you can use static_cast<> to move up (as I have done) but sometimes I think it is clearer to use dynamic_cast<> to move in both directions.

Avoid C style casts in C++ code

C++ has 4 different types of cast designed to replace the C style cast. You are using the equivalent of the reinterpret_cast<> and you are using incorrectly (Any good C++ developer on seeing a reinterpret_cast<> is going to go hold on a second here).

~A3(){
    if(this != (A3*)(A2*)&getPrimaryInstance())
        getPrimaryInstance().unregInst(this);
}

Should be:

~A3()
{
   if (this != dynamic_cast<A3*>(static_cast<A2*>(&getPrimaryInstance())))
   {    getPrimaryInstance().unregInst(this);
   }
}

The layout of an A2 object: (probably something like this).

     Offset   Data
A2   0x00   |------------------
     0x10   * A3 Stuff
            *------------------
     0x20   * A1 Stuff
            *------------------
     0x30   * A2 Stuff

In getPrimaryInstance()

 // Lets assume:
 std::cout << primary; // has an address of 0xFF00

The reference returned will point at the A1 part of the object:

std::cout << &getPrimaryInstancce();
// Should now print out 0xFF20

If you use C style casts it does not check anything just changes the type:

std::cout << (A2*)&getPrimaryInstancce();
// Should now print out 0xFF20
std::cout << (A3*)(A2*)&getPrimaryInstancce();
// Should now print out 0xFF20

Though if you use a C++ cast it should compensate correctly:

std::cout << static_cast<A2*>(&getPrimaryInstance());
// Should now print out 0xFF00
std::cout << dynamic_cast<A3*>(static_cast<A2*>(&getPrimaryInstance()));
// Should now print out 0xFF10

Of course the actual values are all very compiler dependent and will depend on the implementation layout. The above is just an example of what could happen.

Though as pointed out it is probably not safe calling dynamic_cast<> on an object that is currently in the processes of being destroyed.

So how about

Change regInst() so that it returns true for the first object registered. The getPrimaryInstance() will always by the first object to be created so it will allways be the first to register itself.

store this result in a local member variable and only unregister if you are not the first:

A3()
{
    m_IamFirst = getPrimaryInstance().regInst(this);
}

~A3()
{
    if (!m_IamFirst)
    {
         getPrimaryInstance().unregInst(this);
    }
}

Questions:

Why does the this-pointer not point to the where it should(?)?

It does. Just using C-Cast screws up the pointers.

Why does adding virtual to the inheritance like above solve it (despite this still pointing somewhere else than &getPrimaryInstance())?

Because it changes the layout in memory in such a way that the C-Cast is no longer screwing up your pointer.

Is this a bug?

No. Incorrect usage of C-Cast

Can someone try it in a non-MS environment?

It will do somthing similar.

And most importantly: Is this safe??

No. It is implementation defined how virtual members are layed out. You just happen to get lucky.

Solution: Stop using C style casts. Use the appropriate C++ cast.

Upvotes: 4

aschepler
aschepler

Reputation: 72401

class A2 : private A3, public A1 {...} // <-- old version
class A2 : private A3, virtual public A1 {...} //new, with virtual!

Why does adding virtual to the inheritance like above solve it (despite this still pointing somewhere else than &getPrimaryInstance())?

The reason this makes a difference is that virtual inheritance affects the order in which base class constructors and base class destructors are called.

The reason the numeric values of your this pointers aren't the same is that different "base class subobjects" of your complete object A2 primary; can and must have different addresses. Before any destructors are called, you can use dynamic_cast to get between A1* and A2*. When you're certain an A3 object is really the private base part of an A2, you can use a C-style cast to get from A3* to A2*.

But once the body of the destructor ~A2 has completed, which is the case within the ~A3 destructor, a dynamic_cast from A1* to A2* will fail and the C-style cast from A3* to A2* will produce undefined behavior, since there is no longer any A2 object.

So there's probably no way to do what you're trying unless you change how A2 primary; is stored/accessed.

Upvotes: 3

Zac Howland
Zac Howland

Reputation: 15872

There are 2 much bigger questions you should be asking:

  1. Why do I need to use private inheritance?
  2. Why do I need to use multiple inheritance for non-abstract base classes?

In most cases, the answer to #1 is you do not. Declaring a class that contains a data member to the other class usually handles this same situation in a much cleaner and easier to maintain code base.

In most cases, the answer to #2 is also you do not. It is a feature of the language that you use at your own peril.

I recommend you read the Meyers books and reevaluate your design pattern.

Upvotes: 0

ejohansson
ejohansson

Reputation: 2882

When the static A2-instance named primary gets destroyed at program termination the A3-destructor will be called, but inside ~A3 I try to access a function on the same instance that I a destroying. => Access violation at runtime!

When static A2-instance named primary is destroyed the pointer to primary will point to "random" place in the memory. Therefore you are trying to access a random memory location and you get the runtime violation. It all has to do with in what order you call the destructors and make the call in the destructor.

try something like this:

delete a3;
delete a2-primary;

instead of

delete a2-primary;
delete a3;

I also think you might find this typecasting tutorial helpful.

Hope I could help you.

Upvotes: 0

Joris Timmermans
Joris Timmermans

Reputation: 10988

The virtual base class should only come into play if you have a diamond-inheritance structure, i.e. you're multiply inheriting classes sharing a common base class. Are you showing the whole actual inheritance tree of A1, A2 and A3 in your actual code?

Upvotes: 2

engf-010
engf-010

Reputation: 3929

The problem may be that when A3::~A3() is called for an A2 object ,the A2 object is already destructed since ~A3() is called at the end of the destruction of A2. You cannot call getPrimary again since it's destructed already. NB: this applies to the case of the static variable primary

Upvotes: 1

JohnGray
JohnGray

Reputation: 676

Short answer. It happens, because incorrect destructor is calling. For long answer check this and this And check Scott Meyer`s Effective C++. It covers such questions.

Upvotes: -1

Related Questions