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.
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.
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.