OgreResourceProvider memory leak

[nfz]

I've spotted a memory leak while using OgreResourceProvider. I know _Mental_ had hinted at some leaks about a week and a half ago but then mentioned that they were corrected. I've waited since the 9 Dec to see if the fixes would come through on anonymous cvs but still nothing yet. I'll give a quick rundown on what I found since this maybe a different leak.

The code where the leak originates is the following:

Code: Select all

void OgreResourceProvider::loadInputSourceContainer(const String& filename, InputSourceContainer& output)
    {
        Ogre::DataChunk input;
        if( !Ogre::ArchiveManager::getSingleton()._findResourceData(
            filename.c_str(), input ) )
        {
            throw InvalidRequestException((utf8*)
                "Scheme::Scheme - Filename supplied for Scheme loading must be valid");
        }

        XERCES_CPP_NAMESPACE_USE
        InputSource* mInputSource = new MemBufInputSource(input.getPtr(), input.getSize(), filename.c_str(), false);

        output.setData(mInputSource);
    }


When MemBufInputSource is created, false is passed for adoptBuffer. Passing false for adoptBuffer means that when the instance of MemBufInputSource is deleted, it will not delete the buffer passed in the constructor ie input.getPtr(). When "output" is destroyed later on, it only deletes the instance of MemBufInputSource ie the pointer to the instance passed using output.setData(mInputSource) and not the memory that was allocated in Ogre::DataChunk::allocate (line 55: new uchar[mSize + 1];).

I did find a solution but I think either CE or _Mental_ should verify this before I put my foot in my mouth by posting the solution I came up with.

[_mental_]

Yeah setting adoptBuffer to true in this case is the right thing to do. I'll get that changed in CVS tomorrow.

[nfz]

That was my first solution but it causes a bigger problem. Basically its a conflict between Ogre memory manager and Xerces memory manager and causes a crash inside Xerces when it tries to delete the buffer created by Ogre::DataChunk.

My current working solution is a little ugly and involves keeping a second pointer in CEGUI::DataContainer that is set to the datachunk. I know there is a better solution just waiting to poke its head out.

[nfz]

Ok, here is what I am using as a second solution:

I got rid of the typedef for InputSourceContainer and made it a subclass of DataContainer. Here is the class:

Code: Select all

class InputSourceContainer : public DataContainer<XERCES_CPP_NAMESPACE::InputSource>
{

public:

    InputSourceContainer()
        : DataContainer<XERCES_CPP_NAMESPACE::InputSource>()
        , mChunk(0)

    {
    }

    virtual ~InputSourceContainer()
    {
        release();
    }

    void setChunk(unsigned char* chunk) { mChunk = chunk; }

    void release(void)
    {
        if (mChunk)
        {
            delete [] mChunk;
            mChunk = 0;
        }
    }

protected:

    unsigned char* mChunk;

};


the resulting OgreResourceProvider::loadInputSourceContainer looks like this:

Code: Select all

void OgreResourceProvider::loadInputSourceContainer(const String& filename, InputSourceContainer& output)
    {
        Ogre::DataChunk input;
        if( !Ogre::ArchiveManager::getSingleton()._findResourceData(
            filename.c_str(), input ) )
        {
            throw InvalidRequestException((utf8*)
                "Scheme::Scheme - Filename supplied for Scheme loading must be valid");
        }

        XERCES_CPP_NAMESPACE_USE
        InputSource* mInputSource = new MemBufInputSource(input.getPtr(), input.getSize(), filename.c_str(), false);

        output.setData(mInputSource);
        output.setChunk(input.getPtr());
    }


[_mental_]

hmm.. there has to be a better solution. I'll have a think about it.

One thing I did find was that the OGRE memory manager doesn't seem to work on Linux at the moment, so my next step is to fix that up so that I can see what you are talking about If debugging is turned off then the adoptBuffers solution works perfectly.

[CrazyEddie]

For the memory leak the adoptBuffers solution is definately the way to go for clean, 'non-hackish', code. However...

There is a general issue with memory allocated via Ogre with the memory manager on, which is later released in any module without the Ogre memory manager. I've thought about this a bit, and at the moment the only solution is to disable the Ogre memory manager entirely. I am thinking about a couple of other possibilities, but am not saying anything until I have investigated further...

CE.

[CrazyEddie]

There is a general issue with memory allocated via Ogre with the memory manager on, which is later released in any module without the Ogre memory manager.

I have resolved this issue; currently this involves creating a copy of the buffer returned from the Ogre resource manager, the second buffer is allocated outside of the modules using the Ogre memory managers (it's actually allocated in Xerces) so does not suffer any issues when it's later released outside of the Ogre modules.

I do acknowledge that this may not be the ultimate solution, but it does allow the the system to function properly with the Ogre debug memory manager enabled. If a better alternative surfaces, then it will be implemented in place of this current fix.

CE.