hoelzro / inline-lua

Perl extension for embedding Lua scripts into Perl code
4 stars 5 forks source link

Inline::Lua is leaking memory when using a perl sub inside Lua #3

Closed damiens-robert closed 10 years ago

damiens-robert commented 10 years ago

I found out Inline::Lua is leaking memory.

Here is a sample code (note that it uses this project -> https://github.com/henix/slt2 ) :

#!/usr/bin/perl

use Inline Lua => <<LUA;

    -- add /smp/<vhost>/lib to the lua path
    package.path = './?.lua;' .. package.path

    -- load templating library
    local slt2 = require('slt2')

    -- renders the template using the provided vars.
    function render_lua(template, vars, funcs)
        local api = { substring = string.sub, luacall = luacall }
        for n,fn in pairs(funcs) do
          api[n] = fn
        end
        templ = string.gsub(template,string.char(31), ",");
        local tmpl = slt2.loadstring(templ, '<%', '%>')
        return slt2.render(tmpl, vars, api)
    end
LUA

sub mysub {
 print shift." !\n";
}
sub myreturn {
 return shift;
}
my $i = 0;
while($i < 10) {

my $context = {};
$context->{s} = "00010002";
render_lua(
        "<%= subaddr(ext(s)) %>" ,
        $context,
        {
        ext => sub { my $a = shift; my $b = " brol"; return $a.$b."\n"; },
        subaddr => \&myreturn,
        debug => \&mysub,
        }
);
#$i++
}

I execute this command that is finding leaks : perl -MTest::LeakTrace::Script=-verbose testcase.pl

I found a way to fix but I would like your input :)

The SvREFCNT_inc increments the reference counter of the return values of the perl functions and I am not sure if you did it on purpose ... I mean, did you make it to be sure the perl garbage collector wouldn't clear those values or is it a mistake ?

Here is the patch :

--- a/Lua.xs
+++ b/Lua.xs
@@ -86,7 +86,7 @@
     for (i = 0; i < nresults; i++) {
        int offset = nresults - i - 1;
        SV *val = *(sp - offset);
-       SvREFCNT_inc(val);
+//     SvREFCNT_inc(val);
        push_val(L, val);
     }
     /* pop all in one go */
hoelzro commented 10 years ago

@damiens-robert Thanks for the bug report! I've applied your change and pushed it to master. I'll be able to make a release sometime tonight, I think.