[simo.salminen]

Original thread: https://developer.symbian.com/forum/...25311&tstart=0

Please post there if you have account. If not, post here, if you have answer I'll transfer it over to symbian.com afterwards.

[symbianyucca]

because the consytuctor should only contain non leaving code. And this is required for efective memory cleanup. Should be explained pretty well in all books that talk about basic stuff.

[mahbub_s60]

Hi,
Like Jukka mentioned it is described in all books. Two main points are:

C++ constructor can't return any value, we don't have any way to tell caller was it initialized successfully

Partially constructed object can be cleaned up clearly

[wizard_hu_]

Originally Posted by mahbub_s60

C++ constructor can't return any value, we don't have any way to tell caller was it initialized successfully

It is not that easy, neither ConstructL returns a value about success/failure. Generally the pattern appearing in the referred thread does not violate anything trivial.

The reasoning "This might work in some situations, but not in others, because of lifetime issues during the constructor, and rules about the order of base class construction" sounds some kind of nonsense, and pairing the fact of CBase-derived classes are zeroed, and PushL always succeeds in Push-ing - only extending the Cleanup Stack is what may fail - nothing gets leaked, and deletion should occur.

However the post appeared from HamishW (in the referred thread) fades somehow that if multiple pushes occur (in constructors of a class-hierarchy), the multiple deletions may/will panic the code, and that is something that renders the pattern questionable. At least to me.

[nigel.brown]

If "iWotsit=new(ELeave) CWotsit;" leaves, won't iDoodad be orphaned?

Not that this invalidates the pattern, just that bit of code.

As far as I can see, the use of ConstructL is purely so that you can get the naming convention consistent. The constructor doesn't end in "L", so it isn't allowed to leave. This wouldn't be in issue in your own code because you would just "know" what leaves. However, it would be a bit confusing in the SDK if some constructors left and some didn't and there was no way of telling them apart.

[stichbury]

I spoke to Hamish about this yesterday. We felt that the original explanation was somewhat garbled. My take is that, where you have a deep inheritance hierarchy, if you push and pop in each constructor, you end up with inefficient code. For example, if you have:

class C : public class B
{
C()
{CleanupStack::PushL(); DoLeavingStuffL(); CleanupStack::Pop()};
};

class B : public class A
{
B()
{CleanupStack::PushL(); DoLeavingStuffL(); CleanupStack::Pop()};
};


class A : public CBase
{
A()
{CleanupStack::PushL(); DoLeavingStuffL(); CleanupStack::Pop()};
};

When you construct C - you make 3 PushL() calls and 3 Pop() calls.

If you use 2 phase construction, you only push and pop once.

Furthermore, we also thought that it's more intuitive to understand. Two phase construct is an elegant concept and it's clear that it is safe. It's harder to untangle leaving code in the constructor and feel satisified that it is leave-safe (I believe it is, but aren't we aiming to write maintainable, simple code?).

[wizard_hu_]

I am still not satisfied with the explanation.
What happens if C() leaves? Then the Cleanup Stack will begin to release 3 objects (since the 3 PushL-s have happened). This is what I was trying to point out in my previous post.

[wizard_hu_]

...after having lunch...

So there are 3 overlapping objects on the Cleanup Stack - the second Free will be at least "surprised".
On the other hand there are 3 objects on the Cleanup Stack having virtual destructors, so my guess would be that all 3 destructors would be invoked for all three implied PopAndDestroys, definitely killing the code if any of them tries to delete a member variable (quite common for destructors), but does not NULL it (also quite common for destructors, since NULL-ing variables that will not be accessed again - since the object is deleted - is just a waste of code).

[hotcheese]

[quote=nigel.brown;412405]
If "iWotsit=new(ELeave) CWotsit;" leaves, won't iDoodad be orphaned?
QUOTE]


As 'this' is being pushed to the cleanupstack, if it leaves CWidget's destructor will be called.

[simo.salminen]

Thanks all for replies. Trying to get a bottom of this, so did an example. Plot thickens. See below.


This code leaves at C::C(). I put breakpoints in the code, and see that for example C::C() -> C::~C() -> B::~B() -> A::~A(). As expected.

However, after this, there is call C::C() -> B::~B(), thus causing double deletion (USER 42). When debugging, crash does not happen every time, although most of the time.

Why this happens, any ideas?

wizard_hu, I can't see how the cleanupstack could contain the A , B and C this pointers at same time. Each ctor pushes and pops without interfering with others. For example when C::C() pushes 'this', the B and C have already been run (pushed and popped). Right? Remember that base ctors are executed before any code in current class ctor.

class A : public CBase
{
public:
A()
{
CleanupStack::PushL(this); 
iMa = User::AllocL(10);
CleanupStack::Pop(this);
};

virtual ~A()
	{
	delete iMa;
	}
	TAny* iMa;
};

class B : public A
{
public:
B()
{
CleanupStack::PushL(this); 
iMb = User::AllocL(10);
CleanupStack::Pop(this);
};

virtual ~B()
	{
	delete iMb;
	}
	TAny* iMb;
};

class C : public B
{
public:
C()
{
CleanupStack::PushL(this);
iMc = User::AllocL(10);
User::Leave(-666);
CleanupStack::Pop(this);
};

virtual ~C()
	{
	delete iMc;
	}
	TAny* iMc;
};

[wizard_hu_]

Originally Posted by simo.salminen

wizard_hu, I can't see how the cleanupstack could contain the A , B and C this pointers at same time. Each ctor pushes and pops without interfering with others.

You are totally right, I was writing pure nonsense :-(

However, after this, there is call C::C() -> B::~B(), thus causing double deletion (USER 42). When debugging, crash does not happen every time, although most of the time.

Why this happens, any ideas?

If you check the call stack, you will see that C::C() remains there during the whole Cleanup Stack-process. So the first deletion comes from the Cleanup Stack, and the second attempt probably comes from some C++-related mechanism (that is probably why the current class - C - is not included).

Perhaps this pattern is still not that good anyway ;-)

[laa-laa]

Originally Posted by wizard_hu_

So the first deletion comes from the Cleanup Stack, and the second attempt probably comes from some C++-related mechanism (that is probably why the current class - C - is not included).

Yes - nowadays leaves are C++ exceptions internally and not based on setjmp() and longjmp(). When an exception is thrown in a constructor, the destructor is not called since the object has not been constructed yet. However, the superclass parts of the object have been constructed and their destructors are called.

Lauri

[nigel.brown]

Good point.
Won't it cause some potential problems if the cleanupstack tries to delete a partially formed object?

[wizard_hu_]

Yes, I just woke up, and wanted to write what Laa-laa did. So it is not "some C++ mechanism", but the standard behaviour (destructors are called for fully constructed superclasses), and according to some sources (http://www.cs.technion.ac.il/~imaman...owingctor.html) "Initialization exceptions cannot be hidden". Yesterday when I added zeroing+check in the destructors (so deletion iMx-s did not occur twice), User 42 still appeared, since the object itself was still Free-ed twice.
It also means that the construct in question was safe in older Symbian releases (like in its original source - some 92xx-SDK related paper), since that time Leave-s were working independently from C++ exception handling.

[stichbury]

So, to summarize, I think we're saying:

Don't leave in a constructor - use 2-phase construction if you need to.

In Symbian OS v9, this is for an additional reason than the (rather obscure) explanations offered (as quoted in Simo's post).

Correct? Just thought I'd put this here to help future readers of the thread.