Re: [Rlib-devel] Memory leaks

[William K. V. <wk...@us...>]

On Sun, 2005-06-05 at 18:28, Bob Doan wrote:
> Patch looks good.  But there are two problems.
> 
> In resolution.c the strduping needs to stay.  There is no guarantee that
> the pointer returned from data sources is going to stay where its at and
> it since RLIB manipulates some of that data it would be rude to change
> it on the datasource.
> 
> But there is a serious leak there.  Put the strduping back but in
> pcode.c line 738:
> 
> Change:
> 
> return rlib_value_new(rval, RLIB_VALUE_STRING, FALSE,
> rlib_resolve_field_value(r, o->value));
> 
> To:
> 
> return rlib_value_new(rval, RLIB_VALUE_STRING, TRUE,
> rlib_resolve_field_value(r, o->value));
> 
> So it knows to free the string when the RVAL is freed.

Did that and verified that it still doesn't leak.

> Second is rlib_value_dup_contents removal of the strdup.
> 
> There is going to be a double free if it's not duped
> 
> Follow "color" in layout.c:
> 
> extra_data[i].rval_color = line_rval_color; // <--- Really a memset here
> rlib_value_dup_contents(&extra_data[i].rval_color);
> .
> .
> .
> rlib_value_free(&line_rval_color);

I had run test cases, and just verified again that the example case
doesn't fail, even for color (from a previous message):

Well I commented it out, ran some test cases like:
<Line bgcolor="'0xefefef'">
 <literal bgcolor="'0xffffee'" width="95" align="right">Total:</literal>
 <field value="v.subtotal" format="'!$%(n'" width="11" align="right"/>
</Line>

And did not see any problems, no references to freed memory or things
like that.  That said however you are the designer so know what you
intended so I tried your other correction.

> .
> and later the extra data is freed.
> 
> The correct thing to do is probably is set the RVAL to free:
> 
>  struct rlib_value * rlib_value_dup_contents(struct rlib_value *rval) {
>  	if(rval->type == RLIB_VALUE_STRING) {
>  		rval->string_value = g_strdup(rval->string_value);
> 		rval->free = TRUE;
> 	}
>  	return rval;
> }
> 
Tried this and it also worked, the leaking was fixed, this is now
the patch.

> How did you track these down?  Typically I use valgrind to find these
> sorta things.

I first noticed the leaking from the python side and it's garbage
collector.  It's a check I always do when I find out SWIG has
been used (every time I find major memory leaks, some of them
even SWIG related :-)  I then perused the code and, due to having
just poked my nose into a python-libxml2 interface, spotted the
usage of GetProp without the frees.  The leaking was better however
still there from python, code reading found the resolve_fields case.
Today I bit the bullet so to speak and downloaded and fired up
valgrind, generated supressions for python, and verified and
fixed almost all remaining issues.  One left however is in rpdf.c,
the call to rpdf_text is leaving allocations on the slist:

==14555== 13627 bytes in 32 blocks are still reachable in loss record 28
of 32
==14555==    at 0x1B904A80: malloc (vg_replace_malloc.c:131)
==14555==    by 0xAA7A56: g_malloc (in /usr/lib/libglib-2.0.so.0.400.8)
==14555==    by 0xAA8C03: g_mem_chunk_alloc (in
/usr/lib/libglib-2.0.so.0.400.8)
==14555==    by 0xAB675C: (within /usr/lib/libglib-2.0.so.0.400.8)
==14555==    by 0xAB5CB1: g_slist_append (in
/usr/lib/libglib-2.0.so.0.400.8)
==14555==    by 0x1B945607: rpdf_stream_append (rpdf.c:142)
==14555==    by 0x1B94764B: rpdf_text (rpdf.c:798)
==14555==    by 0x1BD74545: pdf_print_text (pdf.c:113)
==14555==    by 0x1BD650EF: rlib_layout_text_string (layout.c:323)
==14555==    by 0x1BD66C9C: rlib_layout_report_output_array
(layout.c:820)
==14555==    by 0x1BD672C7: rlib_layout_report_outputs_across_pages
(layout.c:951)
==14555==    by 0x1BD67302: rlib_layout_report_output (layout.c:961)
==14555==    by 0x1BD63A73: rlib_layout_report (reportgen.c:571)
==14555==    by 0x1BD63F1D: rlib_layout_part_td (reportgen.c:668)
==14555==    by 0x1BD6416A: rlib_layout_part_tr (reportgen.c:715)
==14555==    by 0x1BD64503: rlib_make_report (reportgen.c:800)

Since I'm not a glib guru (actually I've not coded in C since 1997)
I'm not certain what's going wrong here, I added the call
to g_slist_free in rpdf_free however it didn't fix the problem.

Suggestions welcome, otherwise I'll come back to it after I get
the python interface re-done.

btw.  Updated patch with your corrections attached.

Cheers,
William.