Memory Leak in Perl v5.32.1 Regex

[perlscallion]

There's a memory leak here. Perl v5.32.1 on Fedora 33
Just plain code; a subroutine, a regex, no modules

Steps to Reproduce
perl-leak.txt

Perl configuration
perl-V.txt

[perlscallion]

Additional Information
Same code works on Fedora 32 (Perl v5.30.3)
perl-V-5.30.3.txt

Expected Result
The code will run indefinitely, taking 100% of one CPU. Memory use should be stable

Observed Result
With v5.32.1, memory use increases steadily; observed through htop.
The alternative "this doesn't leak" code doesn't leak...

[perlscallion]

Additional Information
Also tested on Fedora 30, Perl v5.28.2, works OK
perl-V-5.28.2.txt

[hvds]

I can confirm the leak, it appears that the culprit is an invlist. With this code:

use Devel::Leak;
my $count = shift @ARGV;

$string = "STRING 1";

my $handle;
Devel::Leak::NoteSV($handle);
for (1 .. $count) {
    $x = test_leak($string, "string 1");
    $x = test_leak($string, "string 2");
}     
Devel::Leak::CheckSV($handle);
exit;

sub test_leak {
    local ($string, $match) = @_;
    $match =~ s/\s/\\s/g;  # This Leaks
    return ($string =~ /$match/i);
}  

.. and comparing the output of running with argument 1 or 2, it appears that an extra two copies of an invlist are created for each iteration, each looking like this:

new 0x55880214d018 : SV = INVLIST(0x55880213b450) at 0x55880214d018
  REFCNT = 1
  FLAGS = (POK,pPOK)
  PV = 0x5588021948a0 
    [0] 0x0085
    [2] 0x00A0
  CUR = 40
  LEN = 41

This appears to have been introduced after v5.30.0, I'll try a bisect.

[jkeenan]

I can confirm the leak, it appears that the culprit is an invlist. With this code:

Hugo, two points:

• Does the leak at all depend on the fact that this code does not work under use strict;?
• When I run your code, I only get this output:

[perlmonger: p5p] $ bleadperl  gh-18604-hvds.pl 1
new 0x800adeba0 : 
new 0x800adbb88 : 
new 0x800adbc78 : 
new 0x800adbcf0 : 
new 0x800a9e810 : 
[perlmonger: p5p] $ bleadperl  gh-18604-hvds.pl 2
new 0x800ade0f0 : 
new 0x800adeba0 : 
new 0x800adede0 : 
new 0x800adbb88 : 
new 0x800adbba0 : 
new 0x800adbc78 : 
new 0x800adbd20 : 

What am I missing? @hvds

Thank you very much.
Jim Keenan

[hvds]

Hugo, two points:

* Does the leak at all depend on the fact that this code does not work under `use strict;`?

No, adding our($string, $match, $x); at the top is enough to make it work under strict, and shows the same results.

* When I run your code, I only get this output:

[...]
What am I missing?

Devel::Leak requires a perl built with debugging enabled for full functionality.

[jkeenan]

* When I run your code, I only get this output:

[perlmonger: p5p] $ bleadperl  gh-18604-hvds.pl 1
new 0x800adeba0 : 
new 0x800adbb88 : 
new 0x800adbc78 : 
new 0x800adbcf0 : 
new 0x800a9e810 : 
[perlmonger: p5p] $ bleadperl  gh-18604-hvds.pl 2
new 0x800ade0f0 : 
new 0x800adeba0 : 
new 0x800adede0 : 
new 0x800adbb88 : 
new 0x800adbba0 : 
new 0x800adbc78 : 
new 0x800adbd20 : 

What am I missing? @hvds

To answer my own (second) question: I need a perl built with -DDEBUGGING. When I then install Devel::Leak, I get output like Hugo's.

[hvds]

I can confirm the leak, it appears that the culprit is an invlist. With this code:

(tightened up a bit from above)

#!/usr/bin/perl
use strict;
use warnings;

use Devel::Leak;
my $count = shift @ARGV;
my $handle;

Devel::Leak::NoteSV($handle);
for (1 .. $count) {
    test_leak('a b');
    test_leak('a c');
}
Devel::Leak::CheckSV($handle);
exit;

sub test_leak {
    my($match) = @_;
    $match =~ s/\s/\\s/g;  
    qr{$match};
    undef;
}

.. and comparing the output of running with argument 1 or 2, it appears that an extra two copies of an invlist are created for each iteration, each looking like this:

new 0x55880214d018 : SV = INVLIST(0x55880213b450) at 0x55880214d018
  REFCNT = 1
  FLAGS = (POK,pPOK)
  PV = 0x5588021948a0 
    [0] 0x0085
    [2] 0x00A0
  CUR = 40
  LEN = 41

This appears to have been introduced after v5.30.0, I'll try a bisect.

Manual bisect points to cfba2ec, @khwilliamson can you take a look?

[hvds]

Oh, it doesn't need s/// either, just qr{} on the same variable with two different strings, that need an invlist:

#!/usr/bin/perl
use strict;
use warnings;

use Devel::Leak;
my $count = shift @ARGV;
my $handle;

Devel::Leak::NoteSV($handle);
for (1 .. $count) {
    test_leak("a\\sb");
    test_leak("a\\sc");
}
Devel::Leak::CheckSV($handle);
exit;

sub test_leak {
    my($match) = @_;
    qr{$match};
    undef;
}

[khwilliamson]

On 2/25/21 8:50 AM, Hugo van der Sanden wrote: Oh, it doesn't need |s///| either, just |qr{}| on the same variable with two different strings, that need an invlist: |#!/usr/bin/perl use strict; use warnings; use Devel::Leak; my $count = shift @ARGV; my $handle; Devel::Leak::NoteSV($handle); for (1 .. $count) { test_leak("a\\sb"); test_leak("a\\sc"); } Devel::Leak::CheckSV($handle); exit; sub test_leak { my($match) = @_; qr{$match}; undef; } |

I found a likely candidate, which caused me to look through that function for others. There were several that would leak if there is a syntax error or warnings are made fatal. I'm thinking the best solution would be to make these things mortal, and not try to catch all possible paths through the code and decrement the reference counts explicitly.

[perlscallion]

... tightened up a bit from above
... Oh, it doesn't need s/// either, just qr{}

Na ja... I missed a few tricks. Nevertheless, good that it could be replicated.
Good luck and Thanks!

[hvds]

I found a likely candidate, which caused me to look through that function for others. There were several that would leak if there is a syntax error or warnings are made fatal. I'm thinking the best solution would be to make these things mortal, and not try to catch all possible paths through the code and decrement the reference counts explicitly.

If there's a risk that such a "best solution" becomes somewhat complex, it may be worth having a commit first to fix the direct leak (that doesn't need syntax errors or fatal warnings), since that will be a priority to get to a maintenance release.

[hvds]

... tightened up a bit from above
... Oh, it doesn't need s/// either, just qr{}

Na ja... I missed a few tricks. Nevertheless, good that it could be replicated.
Good luck and Thanks!

No worries, we appreciate the reports, we have the expertise (usually) to minimize them. :)