Created attachment 99360 [details]
test-case program

Running this simple program makes memory usage (as seen by top) grows rapidly by calling query_child_packing repeatedly.

Yep, I'm seeing the leak too.  Looks like it's due to the usage of xsubpp's OUTLIST feature.  When implemented manually as in the attached patch, the code doesn't leak.

muppet, do you know if that's due to a bug in xsubpp or a misuse of OUTLIST in our code?

Created attachment 99410 [details] [review]
Fix the leak by avoiding OUTLIST

Hrm.  I hate to go changing code all over the place if it's not really necessary.  And i find it hard to believe that OUTLIST would have a leak this bad that nobody has yet noticed.

The generated code goes like this:

        XSprePUSH;       EXTEND(SP,4);
        PUSHs(sv_newmortal());
        ST(0) = boolSV(expand);
        PUSHs(sv_newmortal());
	ST(1) = boolSV(fill);
	PUSHs(sv_newmortal());
	sv_setuv(ST(2), (UV)padding);
	PUSHs(sv_newmortal());
	ST(3) = newSVGtkPackType (pack_type);

In three cases, push a new mortal onto the stack, then discard the pointer to it.  I may be wrong, but i think that the perl garbage collector is not a pure heap scanner, and can only free the scalar if it knows about it.  Even adding a SAVETMPS/FREETMPS pair doesn't seem to do anything.

In xsubpp, the relevant piece of code is:

	    elsif ($do_push) {
	        print "\tPUSHs(sv_newmortal());\n";
		$arg = "ST($num)";
		eval "print qq\a$expr\a";
		warn $@   if  $@;
		print "\tSvSETMAGIC($arg);\n" if $do_setmagic;
	    }

Now, the code for the UV OUTLIST arg padding looks correct --- push a new scalar onto the stack, and alter it.  All of the rest are using typemaps that overwrite the stack value.

But the T_GPERL_GENERIC_WRAPPER uses the form (simplified)

        $arg = newSV$typename ($var);

because it's easy and sensible.  Without changing the convention to saying "hey, you need to provide a set_sv$typename(), or providing a richer set of typemaps and utility functions, then we may be somewhat screwed.

So, it seems that you must take great care with the typemap used for the datatype in the OUTLIST slot.  Perl's built-in typemaps for things like integer, double, and string types seem to be safe.  When in doubt, check the typemap.

Thanks for the clear analysis.  I looked over all the places in Gtk2, where we use OUTLIST for non-standard types and fixed the ones that were leaking memory.

I thought about adding few words about this to the developer docs in Glib but couldn't find a good place where they would fit.