Hi! Andy Wingo <wi...@pobox.com> writes:
> On Tue 22 Feb 2011 12:36, l...@gnu.org (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’.