memory leak

Developer
Oct 9, 2011 at 9:26 AM

Hi Florian,

I observed there are some memory leak warnings when using libopc in MFC.   I wrote a very simple Win32 sample, see it below. My compiler is  VS 2010.

#include <opc/opc.h>
#include <libxml/xmlstring.h>
#include <crtdbg.h>

int main( int argc, const char* argv[] )
{
    // Perform memory leak checking at program exit. 
    _CrtSetDbgFlag (_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
    opcInitLibrary();

    opcContainer* c = NULL;
    c = opcContainerOpen(_X("c:\\OOXMLI1.docx"), OPC_OPEN_READ_WRITE, NULL, NULL);
    opcContainerClose(c, OPC_CLOSE_NOW);
    opcFreeLibrary();
}

Could you please look into it?  Thanks.

Rene

Coordinator
Oct 10, 2011 at 11:40 AM

Hi Rene,

 

thanks for the info. I'll try to have a look soon.

 

Florian

 

Developer
Oct 12, 2011 at 6:47 AM

Hi Florian,

I did some debugging today, and found some problems.  See code below.  Please confirm if the changes I made are correct.

// in zip.c file
opc_error_t opcZipCloseInputStream(opcZip *zip, opcZipInputStream *stream) {
    OPC_ASSERT(NULL!=zip && NULL!=stream);
    OPC_ASSERT(stream->segment_id>=0 && stream->segment_id<zip->segment_items);
    opcZipSegment *segment=&zip->segment_array[stream->segment_id];
    opc_error_t err=opcZipCleanupInflateState(&stream->rawBuffer.state, 
                                              segment->compressed_size, 
                                              segment->uncompressed_size, 
                                              &stream->inflateState);
    // -- added by Rene--.
    if (NULL!=stream) {
        xmlFree(stream);
    }
    return err;
}

 

// in textreader.c file
int mceTextReaderPostprocess(xmlTextReader *reader, mceCtx_t *ctx, int ret) {
    pbool_t suspend=ctx->mce_disabled;
    xmlChar *ns=NULL;
    xmlChar *ln=NULL;
    if (MCE_ERROR_NONE!=ctx->error) {
        ret=-1;
    } else {
        ns=xmlTextReaderNamespaceUri(reader);
        ln=xmlTextReaderLocalName(reader);
        if (XML_READER_TYPE_ELEMENT==xmlTextReaderNodeType(reader)) {
            if (ctx->suspended_level>0 || NULL!=mceQNameLevelLookup(&ctx->suspended_set, ns, ln, PFALSE)) {
                suspend=PTRUE;
                if (!xmlTextReaderIsEmptyElement(reader)) {
                    ctx->suspended_level++;
                }
            }
        } else if (ctx->suspended_level>0) {
            suspend=PTRUE;
            if (XML_READER_TYPE_END_ELEMENT==xmlTextReaderNodeType(reader)) {
                ctx->suspended_level--;
            }
        }
    }
    pbool_t skip=!suspend;
    while(1==ret && skip) {
        if (XML_READER_TYPE_ELEMENT==xmlTextReaderNodeType(reader)) {
            skip=mceTextReaderProcessStartElement(reader, ctx, xmlTextReaderDepth(reader), ns, ln);
            if (xmlTextReaderIsEmptyElement(reader)) {
                PENSURE(mceTextReaderProcessEndElement(reader, ctx, xmlTextReaderDepth(reader), ns, ln)==skip);
            }
        } else if (XML_READER_TYPE_END_ELEMENT==xmlTextReaderNodeType(reader)) {
            skip=mceTextReaderProcessEndElement(reader, ctx, xmlTextReaderDepth(reader), ns, ln);
        } else {
            skip=mceSkipStackSkip(&ctx->skip_stack, xmlTextReaderDepth(reader));
        }
        if (skip) {
            if (-1!=(ret=xmlTextReaderRead(reader))) { // skip element
                // --added by Rene--
                if (NULL!=ns) {
                    xmlFree(ns);
                }
                if (NULL!=ln) {
                    xmlFree(ln);
                }
                ns=xmlTextReaderNamespaceUri(reader); // get next ns
                ln=xmlTextReaderLocalName(reader);  // get net local name
            }
        }
        if (MCE_ERROR_NONE!=ctx->error) {
            ret=-1;
        }
    }
    if (-1==ret && MCE_ERROR_NONE==ctx->error) {
        ctx->error=MCE_ERROR_XML; //@TODO be more specific about the type of libxml2 error.
    }

    // --added by Rene--
    if (NULL!=ns) {
        xmlFree(ns);
    }
    if (NULL!=ln) {
        xmlFree(ln);
    }

    return ret;
}

The memory leak warnings are still displayed, but extremely decreased.



        
    
Developer
Oct 12, 2011 at 7:44 AM
Edited Oct 12, 2011 at 7:50 AM

Another error I found.

// in zip.c file
void opcZipClose(opcZip *zip, opcZipSegmentReleaseCallback* releaseCallback) {
    if (NULL!=zip) {
        if (NULL!=releaseCallback) {
            for(opc_uint32_t i=0;i<zip->segment_items;i++) {
                if (!zip->segment_array[i].deleted_segment) {
                    releaseCallback(zip, i);
                }
            }
        }
        OPC_ASSERT(NULL!=zip->io->_ioclose);
        OPC_ENSURE(0==zip->io->_ioclose(zip->io->iocontext));
        // --added by Rene--
        if (NULL!=zip->segment_array) {
            xmlFree(zip->segment_array);
            zip->segment_array=NULL;
        }
        xmlFree(zip);
    }
}

 

 

If I use OPC_OPEN_READ_ONLY instead of OPC_OPEN_READ_WRITE in my small sample above, there is no memory leak errors after I made these three changes. 

I do suggest to add below code in each sample, so that you can easily find the memory leak problem.

int main( int argc, const char* argv[] )
{
#ifdef WIN32
     _CrtSetDbgFlag (_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
#endif

    // ...

 

Rene

Coordinator
Oct 12, 2011 at 12:37 PM
Edited Oct 12, 2011 at 12:39 PM

Hi Rene,

 

wrt to the second problem. Good catch!!!!!

Funny that I free the memory in the "open" method in case of an error and then forget to free the memory in the close method.

Must have been late ;-) Your solutions look great to me!

 

wrt. to the first problem. I believe I made a mistake in using the correct XML libary method.

By looking at the code I believe I wanted to use "xmlTextReaderConstNamespaceUri" instead of "xmlTextReaderNamespaceUri" and "xmlTextReaderConstLocalName" instead of "xmlTextReaderLocalName".

So my fix would be to replace 

ns=xmlTextReaderNamespaceUri(reader);       

ln=xmlTextReaderLocalName(reader);

with

ns=xmlTextReaderConstNamespaceUri(reader);       

ln=xmlTextReaderConstLocalName(reader);

in the two occasions.

What do you think?

 

P.S.

Since you found and fixed soooo many issues I decided you should have commit rights! I added you as a developer to the project. 

Coordinator
Oct 16, 2011 at 10:35 AM

Hi Rene,

I fixed the issues upstream. See http://libopc.codeplex.com/workitem/662.

I also added the debug statements

#ifdef WIN32
     _CrtSetDbgFlag (_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
#endif
as you requested to detect future problems.
Developer
Oct 17, 2011 at 3:22 AM

Hi Florian,

I was able to update/commit code by using my SVN account, thank you.

I have just fixed two bugs to resolve the memory leak problems occurring in my test application.  Please review my changes and correct me if I made mistakes.

Rene

Coordinator
Oct 18, 2011 at 8:09 AM

Hi Rene,

 

thanks for the fixes!

 

Looking forward for more of your work ;-)

 

Florian