Re: haproxy: new bugreports & patches & ideas

On Sun, 23 Sep 2007, Willy Tarreau wrote:
> On Sun, Sep 23, 2007 at 03:23:38AM +0200, Krzysztof Oledzki wrote:
>> On Sat, 22 Sep 2007, Willy Tarreau wrote:
>>>
>>> I have also noticed it very recently. In fact, it's never the
>>> application (here haproxy) which decides to send an RST, it's the
>>> system. It does so because the server returns data on a terminated
>>> socket. I guess it's because the health-check code does not read much
>>> of the response. I should try to read a full block to see if that
>>> changes. In fact, we just need to read enough to process common
>>> responses. If people are dumb enough to check with something like "GET
>>> /image.iso", they should expect to get an RST after a few kbytes
>>> instead of reading the whole file!
>>>
>> Right, that was easy. Attached patch changed what you described. Now
>> haproxy finishes http checks with FIN:
>
> OK so that confirms that it was indeed the problem. However, I don't like
> your patch because it does malloc() inside I/O routines. What I suggest is
> even simpler : you keep all the recv() and error check path intact, then
> you add a recv() just before out_wakeup, where you inserted a free() :
>
> recv(fd, trash, sizeof(trash), 0);
>
> That will read up to BUFSIZE bytes, ignore the result (since we're not
> interested int it) and nothing needs to be allocated/freed because the
> "trash" buffer is designed exactly for this purpose (declared in haproxy.c
> and include/common/globals.h).
>
> Test it, it should work with only one line changed.

Right, I did not know about this useful trash variable. How about this one? I simply removed the 'reply' variable and changed it into 'trash'.

I believe it is safe to use it that way - this is a single threaded appliaction and the variable is already used in similar way in other places. So, it works as expected with small bonus - less stack usage. ;)

Best regards,

                                 Krzysztof Olêdzki