Top 5 common mistakes to cause memory leaks in Symbian C++ applications

Tutorial posted August 21st, 2007 by jonathan

in

• Basics
• Tips

Platforms:

• S60
• Symbian OS

Keywords:

• Memory Management

Even in a relatively large scale Symbian applications developed in multiple locations by many different people, the different ways to cause memory leaks are not as diverse as one might think.

In fact, here's a list so that once you've read through it you'll be sure to avoid more than two thirds of the most common beginner mistakes and slip ups that cause memory leaks in Symbian C++ applications - in my personal experience.

All of these issues can be avoided all together with a little heads up at the right time.

---

1. R-CLASSES

Using simple RArrays and similar classes carelessly just like automatic variables is a common way to leak memory. For example the following code can look harmless at the first glance, but will leak memory every time it is called:

void CMyClass::DoSomethingWrongL()
{
        RArray<TInt> array;

        iDataBase->GetSelectedIndicesL( array );       
        iListBox->SetItemsSelectedL( array );
}

Even though you do not need to allocate RArrays with new(ELeave) or NewL() and you do not need to call any Open() methods to use them, Close() still needs to be called. Here's the right way to do it:

void CMyClass::DoSomethingRightL()
{
        RArray<TInt> array;

        CleanupClosePushL( array );

        iDataBase->GetSelectedIndicesL( array );       
        iListBox->SetItemsSelectedL( array );

        CleanupStack::PopAndDestroy( &array );

}

Also if you use such arrays as member variables, make sure iArray.Close() gets called in the class destructor.

2. The SizeChanged method

While usings views and containers, be aware of the following:

void CServiceContainer::ConstructL( const TRect& aRect )
    {
        CreateWindowL();

        SetRect( aRect );

        //Create a screensize bitmap to draw on
        iBackgroundBitmap = new(ELeave) CFbsBitmap;
        iBackgroundBitmap->Create( iAppUi->ClientRect().Size(),
                                        CCoeEnv::Static()->ScreenDevice()->DisplayMode() );
        iBackgroundBitmapDevice = CFbsBitmapDevice::NewL( iBackgroundBitmap );
        iBackgroundBitmapDevice->CreateContext( iOSBGc );


        ActivateL();

   }

Looks pretty standart, right? Here's the SizeChanged() method your container needs to implement to support dynamic size changes:

void CServiceContainer::SizeChanged()
    {       
        //delete old bitmap
        delete iBackgroundBitmap; iBackgroundBitmap  = NULL;
        delete iDBOffScreenBitmapDevice; iDBOffScreenBitmapDevice = NULL;
        delete iOSBGc; iOSBGc = NULL;


        //Create a new screensize bitmap
        iBackgroundBitmap = new(ELeave) CFbsBitmap;
        iBackgroundBitmap->Create( iAppUi->ClientRect().Size(),
                                CCoeEnv::Static()->ScreenDevice()->DisplayMode() );
        iBackgroundBitmapDevice = CFbsBitmapDevice::NewL( iBackgroundBitmap );
        iBackgroundBitmapDevice->CreateContext( iOSBGc );
    }

Do you know how memory is leaked here?

The SizeChanged() method is called by the framework whenever the size of the container changes. It is no different with the first SetRect() call in the ConstructL() method.

The SetRect() function call causes a synchronous call to CServiceContainer::SizeChanged(). In the above code snippet, memory is leaked by constructing the bitmap again (without deleting it) in ConstructL() after SetRect() and consequent SizeChanged() call.

So what ever you need to do in SizeChanged(), do it only there.

3. Member variables

Forgetting to add member variables to class destructors.

This is the most obvious cause of leaking memory in object oriented programming lanuguages. Unfortunately it is seldomly the case that the memory leaks in your program are caused in such a simple way.

In any more complex than trivial applications for every member variable it is not right away clear which object owns it. Many types of large objects like bitmaps, buffers or strings need to be referenced from many places instead of making multiple copies.

Comments like "/** not owned */" make things more clear, but cannot be blindly trusted by another person looking at the code, since the more informative and detailed comments are, the sooner they will become outdated when any changes are made.

Use references instead of pointers whenever you can. They leave nothing to the imagination of the person trying to decide whether or not the variable should be mentioned in the class's destructor.

Think of it this way:

1. It takes less than 20 seconds to make sure newly written member variables are closed and deleted properly in the appropriate class's destructor.
2. In the worst case it takes many hours of combined work effort to find, report, fix, test, commit and report such issues as fixed caused by a little too much carelessness at the wrong time.

4. Interface classes

Problems ahead:

MInterface* interface = CImplementation::NewL();
CleanupDeletePushL( interface ); //Use DeletePushL, not a C-class object

interface->FunL();

CleanupStack::PopAndDestroy( interface );

What's wrong with the above code? Nothing... under one strict condition: MInterface class must have virtual destructor implemented.

class MInterface
    {
    public:      
        virtual ~MInterface() {}
              
        virtual FunL() = 0;
    };

Your first reaction might be that it is against the principle of a mixin class (interface class) to have an implementation of anything. That's true, but for "normal" purposes this is by far the most easy and foolproof way to ensure your implementation object gets deleted and released properly.

Problems would arise only if the interface would need to be implemented by a T-class (no destructor) or if for some reason the implementation cannot release all of it's resources in it's destructor. For these purposes instead of having a virtual destructor consider adding, for example, a pure virtual Close() or Release() method to your interface that needs to be called separately when getting rid of the object.

Otherwise in view of multiple inheritance the MInterface class with a virtual destructor implementation is just as dandy.

Destructors are handled a bit specially in C++: they cannot have any parameters and there cannot be two destructors with the same name in two different inherited classes (the names of the classes would need be the same).

Without the virtual destructor, deleting an object through a mixin class pointer leaks memory.

5. Lists' and dialogs' item arrays

Ownership type needs to be set after the array has been set to an object. Here is a common mistake that will leak memory:

CAknListQueryDialog* selectDlg = new(ELeave)CAknListQueryDialog(&item);
selectDlg->PrepareLC(R_SOMETHING);

selectDlg->SetOwnershipType(ELbmDoesNotOwnItemArray);
selectDlg->SetItemTextArray(array);

The correct way to do it is:

selectDlg->SetItemTextArray(array);
selectDlg->SetOwnershipType(ELbmDoesNotOwnItemArray);

This also applies to listboxes:

CTextListBoxModel* model = iListBox->Model();
model->SetItemTextArray( iSettingItemArray );
model->SetOwnershipType( ELbmDoesNotOwnItemArray );

Usually this type of code gets copy-pasted from somewhere, so it should be fine most of the time without paying too much attention to it. But in case it gets done wrong, it can be quite tough to spot if you don't know what you're looking for. It seems logical enough to do it the other way around too.