[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.0-4-g3e0
From: |
Ludovic Courtès |
Subject: |
Re: [Guile-commits] GNU Guile branch, stable-2.0, updated. v2.0.0-4-g3e05fc0 |
Date: |
Tue, 22 Feb 2011 21:35:17 +0100 |
User-agent: |
Gnus/5.110011 (No Gnus v0.11) Emacs/23.2 (gnu/linux) |
Hi!
Andy Wingo <address@hidden> writes:
> On Tue 22 Feb 2011 12:36, address@hidden (Ludovic Courtès) writes:
>
>>>>> +
>>>>> + if (p->input_cd != (iconv_t) -1)
>>>>> + {
>>>>> + iconv_close (p->input_cd);
>>>>> + p->input_cd = (iconv_t) -1;
>>>>> + }
>>>>> +
>>>>> + if (p->output_cd != (iconv_t) -1)
>>>>> + {
>>>>> + iconv_close (p->output_cd);
>>>>> + p->output_cd = (iconv_t) -1;
>>>>> + }
>>>>> +
>>>>
>>>> I don’t think this is needed: each port has a finalizer,
>>>> ‘finalize_port’, which normally takes care of this, eventually.
>>>
>>> It is needed, but only in the case that you `close-port' explicitly.
>>> The block in finalize_port only takes care of gc'd open ports.
>>
>> Right. Closed ports are eventually GC’d, so in that sense it is not
>> strictly needed, but OK.
>>
>> Valgrind was wrong! ;-)
>
> You are setting yourself up for a fall here ;)
Damn, indeed!
> When you close a port via "close-port", you remove the port's
> SCM_PTAB_ENTRY (port). The SCM_PTAB_ENTRY points to the iconv_t, so the
> finalizer would not have a chance to free it, because it can't get to
> it. You need to free the iconv_t at the time you remove the link from
> the port to the SCM_PTAB_ENTRY -- i.e. at close-port time. Otherwise
> you leak the iconv_t.
>
> Trust me: about 500K requests into meta/guile examples/web/hello.scm,
> the memory usage was up at about a gigabyte or so :)
Ouch. Thanks for the explanation, I had clearly overlooked this.
Ludo’.