[p4perl] memory leak in P4Perl and API fix (with patch)

Tony Smith tony at smee.org
Thu Mar 8 01:28:36 PST 2007


Hi John,

Many thanks. I'll take a look at this shortly.

Tony

On Tuesday 06 March 2007 22:49, John LoVerso wrote:
> Sandy Currier and I found two problems in P4Perl that the stable
> version (3.5708) and in the devel tip (3.5787).
>
> 1. As documented in the changelog for 3.5038, this leaks like a sieve:
>         while (1) {
>             my $p4 = new P4;
>         }
>
> It leaks just as much when making multiple requests the same P4 object,
> as in:
>
>         while (1) {
>             ...
>             $res = $p4->changes('-m2')
>             ...
>         }
>
> The leak is caused by the use of 'newAV()' in P4Result::Reset(), since
> this
> fails to free the previous values of the 3 arrays (really the AV * array
> head, since the contents of the array have been cleared by the
> av_undef).
> This could be fixed by just removing the calls to newAV(), but that
> brings us to...
>
>
> 2. The second problem is that each successive call to a P4 object
> destroys
> the previous return value.  You can see this in:
>
>         my $p4 = new P4;
>         $p4->Tagged();
>         $p4->Connect();
>
>         my $res1 = $p4->changes('-m3');
>         print "result1 has ".scalar(@$res1)." entries\n";
>
>         my $res2 = $p4->users('-m2');
>         print "result2 has ".scalar(@$res2)." entries\n";
>
>         print "result1 has ".scalar(@$res1)." entries\n";
>
> This displays:
>
>         result1 has 3 entries
>         result2 has 2 entries
>         result1 has 0 entries
>
> That's because of the same code path through P4Result::Reset(), which
> ends up calling av_undef() via P4Result::Clear().  PerlClientApi::Run()
> returns a new reference to the output array collected in P4Result, and
> so the call to av_undef() actually destroys the previously returned
> value.
>
> The fix to this is just as simple: instead of calling av_undef(),
> call sv_free() to just decrement the reference count being held on the
> arrays in the P4Result object.  If we've already returned a reference
> on the output array to the user, their reference count will keep their
> copy in existence.  If not, the array will be reclaimed by Perl.
> The call to newAV() gets a fresh array for the next operation.
>
>
> The combined fix for both of the above is in the attached patch.
>
> John & Sandy



More information about the p4perl mailing list