On 3/21/2011 11:04 AM, Jacek Masiulaniec wrote:
On 19 March 2011 20:04, Jonny Mosco<[email protected]> wrote:
I'm using smtpd as a smtp relay for around 50+ systems. It will run for a
few days and eat up memory before crashing. The patch provided is by Jared
Yanovich. After a good amount of testing, we discovered the memory leak.
Index: asr.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/asr.c,v
retrieving revision 1.2
diff -u -p -r1.2 asr.c
--- asr.c 3 Dec 2010 23:29:08 -0000 1.2
+++ asr.c 19 Mar 2011 19:48:33 -0000
@@ -774,8 +774,7 @@ asr_ctx_query(struct asr_ctx *ac, int ty
void
asr_done(struct asr *asr)
{
- if (asr_ctx_unref(asr->a_ctx))
- return;
+ asr_ctx_unref(asr->a_ctx);
if (asr->a_path)
free(asr->a_path);
free(asr);
Index: client.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/client.c,v
retrieving revision 1.33
diff -u -p -r1.33 client.c
--- client.c 28 Nov 2010 13:56:43 -0000 1.33
+++ client.c 19 Mar 2011 19:48:33 -0000
@@ -573,6 +573,7 @@ client_close(struct smtp_client *sp)
SSL_free(sp->ssl);
#endif
close(sp->w.fd);
+ fclose(sp->body);
free(sp);
}
Client API's caller manages the message body descriptor so close()
belongs there. I believe the caller has been releasing this
descriptor correctly before this diff.
The problem is that, roughly speaking, fclose does free+close, while
what is needed here is just free. The solution is to use something
else than stdio, or to change the API so that caller no longer manages
the descriptor.
That is a pretty strange behavior, when doing fd-passing using the imsg
API, it becomes imsg's duty to release the fd. I think the client API
should follow the same rule, once you hand it over a body descriptor, it
should take care of closing/releasing properly the resource (that is the
stdio stream it has allocated with fdopen AND the body fd it was handed
over). It currently takes care of releasing everything BUT the fd.
Gilles
