On Mon, Jun 08, 2015 at 09:12:32PM +0200, Joerg Jung wrote: > 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 <[email protected]>: > > > > > > > > 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?
Sure. Fine with me. Wondering if the -1 check is needed. IIRC close(-1); is save. Anyway you want to add a space after the if. > 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 > > > -- :wq Claudio
