I am not sure this line of attack is correct.
1. if there are 2 or more STRING values in a single pmResult the
callback is called multiple times with the strdup'd value locked in the
pmResult ... so the free() at best releases the last value only.
2. even more critically, I don't see where the problem lies (in which
case the fix, even if 1. is accounted for, is heading towards freeing
already free'd memory).
In the normal course of events (for non Perl PMDAs), the PMDA callback
is called once per metric-instance pair ... this can lead to a bunch of
malloc'd memory being held in the pmResult ... this is done via the
__pmStuffValue call after each callback return (with the pmAtomValue set
in the callback). __pmStuffValue will return PM_VAL_DPTR in the case of
a STRING data type (or AGGREGATE, but that is irrelevant here) and this
is used to set valfmt in the pmResult.
Now the _Result_ callback is __pmFreeResultValues and this happens
(pmda->e_resultCallBack)(result);
in __pmdaMainPDU after the pmResult has been sent to PMCD.
And finally, __pmFreeResultValues _should_ end up calling
free(pvs->vlist[j].value.pval);
if this is true ...
if (pvs->numval > 0 && pvs->valfmt == PM_VAL_DPTR)
I can't see anything in the Perl PMDA wrapper that would prevent this
behaviour.
I have build a dummy Perl PMDA and traced through this and I am seeing
the free() call as expected.
So, I'd like to see the test case that demonstrates the leak, along with
the output from running the PMDA with -Dpdubuf which should give good
evidence.
Attached is my evidence of things working as expected with my "bozo"
PMDA (see the <=== HERE annotation).
On Sun, 2010-08-01 at 09:52 +1000, Nathan Scott wrote:
> Changes committed to git://oss.sgi.com/pcp/pcp.git dev
>
> src/cpan/PMDA/Changes | 3 +++
> src/cpan/PMDA/PMDA.pm | 2 +-
> src/cpan/PMDA/PMDA.xs | 17 +++++++++++++++--
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> commit 4f2dd19e7695497ee01123215dae2f901d5d1834
> Author: Jason Rappleye <jason.rappleye@xxxxxxxx>
> Date: Sun Aug 1 09:39:53 2010 +1000
>
> Fix a memory leak in Perl PMDA wrapper string handling.
>
> (nathans modified original patch slightly to ensure that
> the string is always freed, even on final fetch callback)
>
> _______________________________________________
> pcp mailing list
> pcp@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/pcp
bozo.log
Description: Text Data
|