On Tue, Jun 02, 2015 at 05:47:47PM +0200, Claudio Jeker wrote: > On Tue, Jun 02, 2015 at 01:50:35PM +0200, Joerg Jung wrote: > > > > > Am 01.06.2015 um 01:25 schrieb Todd Mortimer <t...@opennet.ca>: > > > > > > I agree that my patch is more of a workaround, and it would be > > > better to track down how it is that the client is being passed to > > > server_fcgi with an open socket. I was going this way when I started > > > looking at the source, but then I saw that clt->clt_srvevb and > > > clt->clt_srvbev get the same treatment (free if not null, then > > > reassign) at the same spot in server_fcgi(), and I figured if it > > > was good enough for clt_srvevb and clt_srvbev, why not for clt_fd? > > > > Yes, you are right. I think your proposed diff is correct. > > I would like to commit it, if anyone is willing to give OK. > > This feels to me more like a workaround. Since what happens is that a > connection is either reused without cleanup or we call the connection > establishment multiple time for the same client. > relayd had a similar issue that I fixed lately. One of the issues is that > event callbacks can be called when you don't really expect them to be > called.
Yes, workaround was my first impression as well. But after staring at the code for a while, I think fixing it in the "right way" seems not trivial and involves several changes. So, since the diff below is simple and goes along with the style of the existing code, AND fixes an actual leak, I would suggest to commit it for now, until someone comes up with something better? Regards, Joerg > > > > > > Index: server_fcgi.c > > > =================================================================== > > > RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v > > > retrieving revision 1.53 > > > diff -u -p -u -p -r1.53 server_fcgi.c > > > --- server_fcgi.c 26 Mar 2015 09:01:51 -0000 1.53 > > > +++ server_fcgi.c 31 May 2015 22:33:54 -0000 > > > @@ -31,6 +31,7 @@ > > > #include <stdio.h> > > > #include <time.h> > > > #include <ctype.h> > > > +#include <unistd.h> > > > #include <event.h> > > > > > > #include "httpd.h" > > > @@ -152,6 +153,9 @@ server_fcgi(struct httpd *env, struct cl > > > errstr = "failed to allocate evbuffer"; > > > goto fail; > > > } > > > + > > > + if(clt->clt_fd != -1) > > > + close(clt->clt_fd); > > > > > > clt->clt_fd = fd; > > > if (clt->clt_srvbev != NULL) > > > > > > > -- > :wq Claudio >
