File descriptor leak in mhfixmsg?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

File descriptor leak in mhfixmsg?

Valdis Klētnieks
After the recent discussion about mhfixmsg, I decided I was *totally*
fed up with the fact that grep kept missing things that were inside
base64 text/* blocks.

So trying to fix up a chunk of inbox died:

% mhfixmsg +inbox 1:9999 -verbose -decodetext 8bit
(....)
mhfixmsg: 3955, fix multipart boundary
mhfixmsg: /home/valdis/Mail/mhfixmsgWNiosi part 1, insert text/plain part
mhfixmsg: 3956, insert text/plain part
mhfixmsg: 3957, insert text/plain part
mhfixmsg: 3959, insert text/plain part
mhfixmsg: 3960, insert text/plain part
mhfixmsg: 3962, fix multipart boundary
mhfixmsg: /home/valdis/Mail/mhfixmsgPPSDsD part 1, will not decode because it is binary (null character)
mhfixmsg: /home/valdis/Mail/mhfixmsgPPSDsD part 2, decode text/plain; charset=US-ASCII
mhfixmsg: 3963, insert text/plain part
mhfixmsg: 3964, insert text/plain part
mhfixmsg: unable to open for writing /home/valdis/Mail/mhfixmsg6zYphW: Too many open files
mhfixmsg: unable to replace broken boundary
mhfixmsg: unable to create temporary file in /home/valdis/Mail
mhfixmsg: unable to create temporary file in /home/valdis/Mail


_______________________________________________
Nmh-workers mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/nmh-workers

attachment0 (866 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: File descriptor leak in mhfixmsg?

David Levine-3
Valdis wrote:

> mhfixmsg: unable to open for writing /home/valdis/Mail/mhfixmsg6zYphW: Too many open files

Well, I can't say that I'm surprised, but of course this shouldn't happen.  mhfixmsg uses the MH MIME parser and it likes tmp files.  I tried to be careful with mhfixmsg but it's possible I missed something.

Though, I just ran it over a huge folder, too, under valgrind --track-fds=yes.  It didn't report any fd leaks.

What rmmproc are you using?  Maybe it's something related to that.

I also noticed that there was a bug in this default rule in mhn.defaults:

    mhfixmsg-format-text/calendar: mhical -infile %F

It was missing the "-infile ".  I committed a fix.

David

_______________________________________________
Nmh-workers mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/nmh-workers
Reply | Threaded
Open this post in threaded view
|

Re: File descriptor leak in mhfixmsg?

Conrad Hughes
[Thanks for your quick reply yesterday David!]

So, resurrecting a 4-year-old thread here..  I'm getting "Too many open
files" errors running mhfixmsg on folders of over around 1020 messages,
congruent to my descriptors limit of 1024.

David, you earlier (like, January 2016 earlier) said you couldn't
replicate this with valgrind, but when I run

  valgrind --leak-check=full --track-fds=yes uip/mhfixmsg -verbose +z all

from the latest git sources off Savannah, I find all the leaked
descriptors to come from mhfixmsgsbr:

  ==27025== Open file descriptor 5: /home/conrad/.Mail/mhfixmsgrSu5Zs
  ==27025==    at 0x4B861AE: open (open64.c:48)
  ==27025==    by 0x4B095C2: __gen_tempname (tempname.c:261)
  ==27025==    by 0x129DB9: m_mktemp (m_mktemp.c:64)
  ==27025==    by 0x10EF61: mhfixmsgsbr (mhfixmsg.c:604)
  ==27025==    by 0x10EF61: main (mhfixmsg.c:541)

It *looks* to me as if the declaration in mhfixmsgsbr() that outfp will
be closed by its caller is not strictly true: if I understand correctly,
a new file is opened every time the cts/ctp loop on line 540 iterates,
where the only *close* is a singular one on line 564.  Adding a line 554
along the lines of

  if (outfp) { fclose (outfp); outfp = NULL; }

.. appears to solve things for me without breaking any tests; I'm unsure
whether line 564 needs to remain for other code paths though.  Seems
*possible* there was a no-longer-relevant design decision to relinquish
control over outfp, and it could perhaps now be closed in mhfixmsgsbr()?

Conrad

Reply | Threaded
Open this post in threaded view
|

Re: File descriptor leak in mhfixmsg?

David Levine-3
Conrad writes:

> It *looks* to me as if the declaration in mhfixmsgsbr() that outfp will
> be closed by its caller is not strictly true: if I understand correctly,
> a new file is opened every time the cts/ctp loop on line 540 iterates,
> where the only *close* is a singular one on line 564.  Adding a line 554
> along the lines of
>
>   if (outfp) { fclose (outfp); outfp = NULL; }
>
> .. appears to solve things for me without breaking any tests;

Fix committed, thank you!

> Seems
> *possible* there was a no-longer-relevant design decision to relinquish
> control over outfp, and it could perhaps now be closed in mhfixmsgsbr()?

The idea was to allow mhfixmsgsbr() to be callable by other programs.  It
in fact isn't, but that's consistent with other MH usage of having the
main program call a sbr.

David