On Tue, Jun 09, 2015 at 07:46:15AM +0000, Florian Obser wrote: > On Mon, Jun 08, 2015 at 09:17:41PM +0200, Claudio Jeker wrote: > > 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 <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? > > > > 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. > > > > OK for me as well. Please go ahead. I was hoping to look into it and > find the "correct" fix but I didn't get around to it. So this > workaround is better than nothing I guess.
Committed, thanks. Regards, Joerg
