Re: Is this a memory leak?

"Hwang, Johnny" <[email protected]> Thu, 27 Sep 2012 03:31:13 +0000
Newsgroups gmane.network.open-pegasus.general
Message-ID <[email protected]>
Thanks for telling me nothing useful. We don't use the ssl stack in our provider. Only Pegasus uses it.

If you have nothing interesting to add then stfu and gtfo. Reading your email has always been a waste of brain and I'm getting sick and tired of it.


Sent via the Samsung Galaxy S™III, an AT&T 4G LTE smartphone

Kirk Augustin <[email protected]> wrote:
Lets not get carried away.
No tool like Valgrind can ever trace or detect memory leaks between multiple processes.
Linux in general rarely has serious memory leaks because it is the operating system that tracks memory use.
A deliberate failure to free accumulating resources like indications is not a memory leak, but simply bad client usage.
In contrast, Windows does not track resources in the operating system, but in the prolog of each process, so constantly has massive memory leaks.
COM sidesteps process prologs, so makes resource tracking impossible in Windows.
So when one discusses memory leaks, one should try to be reasonable.

For example, there is no reason for SSL to retain any memory once authentication has been made, so it seem the library would be unloaded and memory all freed.
In fact, there seems no reason for indications to be made secure at all, for an indication should not be a command but merely a hint something needs to be looked at.
The implication of complexity that would make for memory loss to be possible would inherently be a programming flaw.
It is not that memory leaks should be fixed, but that programming complexity that could allow for memory leaks should never be allowed.

To restate again, memory leaks are not a problem to fix.
Memory leaks are an indicator of overly complex programming models, that are wrong and need to be simplified.
Good programming is simple to follow and won't allow significant memory leaks.

Kirk  Augustin
11821 NW McNamee Rd
Portland, OR 97231

HM: 503-289-4356
________________________________
From: "Hwang, Johnny" <[email protected]>
To: "[email protected]" <[email protected]>
Cc: "[email protected]" <[email protected]>
Sent: Wednesday, September 26, 2012 5:41 PM
Subject: RE: Is this a memory leak?

Hello all again,

I have found a new memory leak, this time in calling the openssl library. This is after sending roughly ~2500 indications over a period of time (1 hour). Here is the leak trace:

==3226== 13,112 bytes in 14 blocks are possibly lost in loss record 4,875 of 4,972
==3226==    at 0x4C710D5: operator new(unsigned int) (vg_replace_malloc.c:214)
==3226==    …false positive…
==3226==
==3226== 30,324 bytes in 2,527 blocks are possibly lost in loss record 4,907 of 4,972
==3226==    at 0x4C70903: malloc (vg_replace_malloc.c:195)
==3226==    by 0x89907A1: default_malloc_ex (in /usr/ontap/smis/pegasus/lib/libcrypto.so.1.0.0)
==3226==
==3226== 32,768 bytes in 1 blocks are possibly lost in loss record 4,915 of 4,972
==3226==    at 0x4C70A02: realloc (vg_replace_malloc.c:476)
==3226==    …false positive…
==3226==
==3226== LEAK SUMMARY:
==3226==    definitely lost: 25,722 bytes in 928 blocks
==3226==    indirectly lost: 2,942 bytes in 113 blocks
==3226==      possibly lost: 158,823 bytes in 3,744 blocks
==3226==    still reachable: 28,012,358 bytes in 147,001 blocks
==3226==         suppressed: 0 bytes in 0 blocks
==3226== Reachable blocks (those to which a pointer was found) are not shown.
==3226== To see them, rerun with: --leak-check=full --show-reachable=yes
==3226==
==3226== Use --track-origins=yes to see where uninitialised values come from
==3226== ERROR SUMMARY: 57998 errors from 1376 contexts (suppressed: 5440 from 13)

Here is the code snippet in Pegasus (SSLContext.h):

However, further digging today revealed what looks potentially like another source of memory leak, this time in SSLContext.h. I will be copy+pasting the below information in another thread to the pegasus group, so sorry for the spam.

Here's the code snippet:
-------------------------------------------
    SSLEnvironmentInitializer()
    {
        AutoMutex autoMut(_instanceCountMutex);

        PEG_TRACE((TRC_SSL, Tracer::LEVEL4,
            "In SSLEnvironmentInitializer(), _instanceCount is %d",
            _instanceCount));

        if (_instanceCount == 0)
        {
            _initializeCallbacks();
            SSL_load_error_strings();
            SSL_library_init();
        }

        _instanceCount++;
    }

    ~SSLEnvironmentInitializer()
    {
        AutoMutex autoMut(_instanceCountMutex);
        _instanceCount--;

        PEG_TRACE((TRC_SSL, Tracer::LEVEL4,
            "In ~SSLEnvironmentInitializer(), _instanceCount is %d",
            _instanceCount));

        if (_instanceCount == 0)
        {
            ERR_free_strings();
            _uninitializeCallbacks();
        }
    }
-------------------------------------------

Here's something I read on stackoverflow:
http://stackoverflow.com/questions/11759725/opensslssl-library-init-memory-leak

Need to call these functions to free memory:

CONF_modules_free();
ERR_remove_state(0);
ENGINE_cleanup();
CONF_modules_unload(1);
ERR_free_strings();
EVP_cleanup();
CRYPTO_cleanup_all_ex_data();

Need to call this function for each thread:

ERR_remove_state(0)


Would that be a memory leak? =D


From: Hwang, Johnny
Sent: Sunday, September 23, 2012 11:22 PM
To: Hwang, Johnny; [email protected]
Cc: [email protected]
Subject: RE: Is this a memory leak?

Hello all,

I have retrieved all the information I could and can conclusively conclude (as far as I understand) that there is no memory leak in that part of the pegasus code.
Just a reminder to all developers:
If you have a provider that contains an active thread that runs in a loop that directly or indirectly allocates memory from CMPI and/or calls helper functions that directly or indirectly allocates memory from CMPI, all those memory must be explicitly allocated.

Example calls:
newInstance()
getKey() [if the return value is a CMPIString, CMPI_ref, etc., something other than numbers]
toString()
getClassName()

In addition to clone(), which is a known (explicit) memory allocator.

Could I also get a complete list of implicit memory allocators, so that I know which functions to watch out for as potential memory leaks?

Thanks,
Johnny

From: Hwang, Johnny
Sent: Sunday, September 23, 2012 11:17 AM
To: [email protected]; Hwang, Johnny
Cc: [email protected]
Subject: Re: Is this a memory leak?

Hmm I am calling release() on the instance with which I called deliverIndication(). The leak trace is not pointing to newInstance(), but to deliverInstance(). I'll try to get more information on this.


Sent via the Samsung Galaxy S™III, an AT&T 4G LTE smartphone

Marek Szermutzky <[email protected]<mailto:[email protected]>> wrote:
This is not a memory leak. Please let me explain why and where the memory is released.

The object holding the memory created through _rep = new CIMInstanceRep() is CIMInstance indInst;
That object is placed on the stack, meaning on leaving function mbDeliverIndication() in CMPIBroker.cpp the destructor of it is called.
The destructor of CIMInstance does call Dec(_rep) which is inherited from class CIMObjectRep.
    void Dec()
    {
        if (_refCounter.decAndTestIfZero())
            delete this;
    }
and here the internal representation of object CIMInstanceRep is deleted.

If you see memory growing on indication providers, a common mistake made by CMPI provider writers is not to release instances delivered while delivering instances through a separate thread created by the provider for that purpose. Calling CMReleaseInstance() after each CMDeliverInstance() will help the CIM Server to release that memory early.


Mit freundlichen Grüßen / Kind regards
Marek Szermutzky

Software Engineer / OpenPegasus Maintainer (PMC) and z/OS PlatformRep.
IBM Systems &Technology Group, Systems Software Development / z/OS Capacity Management and Support
-------------------------------------------------------------------------------------------------------------------------------------------
IBM Deutschland
Schoenaicher Str. 220
71032 Boeblingen
Phone: +49-7031-16-5182
E-Mail: [email protected]<mailto:[email protected]>
-------------------------------------------------------------------------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH / Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294



From:        "Hwang, Johnny" <[email protected]<mailto:[email protected]>>
To:        "[email protected]<mailto:[email protected]>" <[email protected]<mailto:[email protected]>>
Date:        19.09.2012 10:07
Subject:        Is this a memory leak?
________________________________



This is in src/Pegasus/ProviderManager2/CMPI/CMPI_Broker.cpp, circa line 900.

    static CMPIStatus mbDeliverIndication(
        const CMPIBroker* eMb,
        const CMPIContext* ctx,
        const char *ns,
        const CMPIInstance* ind)
    {
        PEG_METHOD_ENTER(
            TRC_CMPIPROVIDERINTERFACE,
            "CMPI_Broker:mbDeliverIndication()");
        // If no valid broker was passed in we try the use the broker
        // that was stored in the local thread context
        if (eMb==NULL)
        {
        eMb = CM_BROKER;
        }
        CMPI_Broker *mb = (CMPI_Broker*)eMb;
        IndProvRecord *indProvRec;
        OperationContext* context = CM_Context(ctx);

        SCMOInstance* scmoInst = SCMO_Instance(ind);
        CIMInstance indInst;
        scmoInst->getCIMInstance(indInst);  //*** <- IS THIS A MEMORY LEAK?!?!? Does this guy need to be freed explicitly? getCIMInstance() calls new() in its stack.***

…
        PEG_METHOD_EXIT();
        CMReturn(CMPI_RC_ERR_FAILED);
    }
}


This is my valgrind feed:
> ==14407== 71,088 bytes in 1,481 blocks are possibly lost in loss record 4,369 of 4,489
> ==14407==    at 0x4C710D5: operator new(unsigned int) (vg_replace_malloc.c:214)
> ==14407==    by 0x7A65AAE: Pegasus::CIMInstanceRep::clone() const (CIMInstanceRep.h:64)
> ==14407==    by 0x7A81101: Pegasus::CIMObject::clone() const (CIMObject.cpp:229)
> ==14407==    by 0x7AA5A0F: Pegasus::CIMValue::set(Pegasus::CIMObject const&) (CIMValue.cpp:797)
> ==14407==    by 0x7A22575: Pegasus::SCMOInstance::_getCIMValueFromSCMBUnion(Pegasus::CIMValue&, Pegasus::CIMType, bool, bool, unsigned int, Pegasus::SCMBUnion const&, char const*) (SCMO.cpp:2175)
> ==14407==    by 0x7A22919: Pegasus::SCMOInstance::_getCIMValueFromSCMBValue(Pegasus::CIMValue&, Pegasus::SCMBValue const&, char const*) (SCMO.cpp:2244)
> ==14407==    by 0x7A22BEC: Pegasus::SCMOInstance::_getCIMPropertyAtNodeIndex(unsigned int) const (SCMO.cpp:1747)
> ==14407==    by 0x7A23B1A: Pegasus::SCMOInstance::getCIMInstance(Pegasus::CIMInstance&) const (SCMO.cpp:1633)
> ==14407==    by 0xC0CE632: mbDeliverIndication (CMPI_Broker.cpp:928)
> ==14407==    by 0xC12BD14: sendIndicationFunction (x.c:x)<- this line reads "pBrk->bft->deliverIndication(pBrk, pCtx, pNamespace,…)"
> ==14407==

I have nowhere near 1481 indication items to process. More in the order of low hundreds, run for several cycles. Therefore, this is not an individual cycle, but continuously piled up over time and is most likely a leak.

Thanks for understanding,
Johnny Hwang