From: Willy Tarreau Date: Sun, 23 Nov 2008 18:31:35 +0000 (+0100) Subject: [MEDIUM] remove stream_sock_update_data() X-Git-Tag: v1.3.16-rc1~143 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0a5d5ddeb98f37149c04e6f729f0320b9a15957c;p=thirdparty%2Fhaproxy.git [MEDIUM] remove stream_sock_update_data() Two new functions are used instead : buffer_check_{shutr,shutw}. It is indeed more adequate to check for new closures only when the buffer reports them. Several remaining unclosed connections were detected after a test, even before this patch, so a bug remains. To reproduce, try the following during 30 seconds : inject30l4 -n 20000 -l -t 1000 -P 10 -o 4 -u 100 -s 100 -G 127.0.0.1:8000/ --- diff --git a/include/proto/buffers.h b/include/proto/buffers.h index 36a82d8de4..7d1bb11ac1 100644 --- a/include/proto/buffers.h +++ b/include/proto/buffers.h @@ -3,7 +3,7 @@ Buffer management definitions, macros and inline functions. Copyright (C) 2000-2008 Willy Tarreau - w@1wt.eu - + This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation, version 2.1 @@ -150,6 +150,46 @@ static inline void buffer_write_dis(struct buffer *buf) buf->flags &= ~BF_WRITE_ENA; } +/* check if the buffer needs to be shut down for read, and perform the shutdown + * at the stream_interface level if needed. This must not be used with a buffer + * for which a connection is currently in queue or turn-around. + */ +static inline void buffer_check_shutr(struct buffer *b) +{ + if (b->flags & BF_SHUTR) + return; + + if (!(b->flags & (BF_SHUTR_NOW|BF_SHUTW))) + return; + + /* Last read, forced read-shutdown, or other end closed. We have to + * close our read side and inform the stream_interface. + */ + buffer_shutr(b); + b->prod->shutr(b->prod); +} + +/* check if the buffer needs to be shut down for write, and perform the shutdown + * at the stream_interface level if needed. This must not be used with a buffer + * for which a connection is currently in queue or turn-around. + */ +static inline void buffer_check_shutw(struct buffer *b) +{ + if (b->flags & BF_SHUTW) + return; + + if ((b->flags & BF_SHUTW_NOW) || + (b->flags & (BF_EMPTY|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR)) == + (BF_EMPTY|BF_WRITE_ENA|BF_SHUTR)) { + /* Application requested write-shutdown, or other end closed + * with empty buffer. We have to close our write side and + * inform the stream_interface. + */ + buffer_shutw(b); + b->cons->shutw(b->cons); + } +} + /* returns the maximum number of bytes writable at once in this buffer */ static inline int buffer_max(const struct buffer *buf) { diff --git a/include/proto/stream_sock.h b/include/proto/stream_sock.h index ddad638254..a7bcb9937b 100644 --- a/include/proto/stream_sock.h +++ b/include/proto/stream_sock.h @@ -33,10 +33,9 @@ /* main event functions used to move data between sockets and buffers */ int stream_sock_read(int fd); int stream_sock_write(int fd); -int stream_sock_data_update(int fd); int stream_sock_data_finish(int fd); -int stream_sock_shutr(struct stream_interface *si); -int stream_sock_shutw(struct stream_interface *si); +void stream_sock_shutr(struct stream_interface *si); +void stream_sock_shutw(struct stream_interface *si); /* This either returns the sockname or the original destination address. Code diff --git a/include/types/buffers.h b/include/types/buffers.h index 7e72c28e1d..ddfb2771cf 100644 --- a/include/types/buffers.h +++ b/include/types/buffers.h @@ -3,7 +3,7 @@ Buffer management definitions, macros and inline functions. Copyright (C) 2000-2008 Willy Tarreau - w@1wt.eu - + This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation, version 2.1 @@ -24,6 +24,7 @@ #include #include +#include /* The BF_* macros designate Buffer Flags, which may be ORed in the bit field * member 'flags' in struct buffer. Here we have several types of flags : diff --git a/include/types/stream_interface.h b/include/types/stream_interface.h index 111ffc740c..ffaf8ad65c 100644 --- a/include/types/stream_interface.h +++ b/include/types/stream_interface.h @@ -74,8 +74,8 @@ struct stream_interface { int fd; /* file descriptor for a stream driver when known */ unsigned int flags; /* SI_FL_*, must be cleared before I/O */ unsigned int exp; /* wake up time for connect, queue, turn-around, ... */ - int (*shutr)(struct stream_interface *); /* shutr function */ - int (*shutw)(struct stream_interface *); /* shutw function */ + void (*shutr)(struct stream_interface *); /* shutr function */ + void (*shutw)(struct stream_interface *); /* shutw function */ struct buffer *ib, *ob; /* input and output buffers */ unsigned int err_type; /* first error detected, one of SI_ET_* */ void *err_loc; /* commonly the server, NULL when SI_ET_NONE */ diff --git a/src/proto_http.c b/src/proto_http.c index df905e2713..f73964a3c5 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -1136,7 +1136,12 @@ void process_session(struct task *t, int *next) if (s->rep->cons->state != SI_ST_CLO) { if (((rqf_cli ^ s->req->flags) & BF_MASK_INTERFACE_I) || ((rpf_cli ^ s->rep->flags) & BF_MASK_INTERFACE_O)) { - stream_sock_data_update(s->rep->cons->fd); + + if (!(s->rep->flags & BF_SHUTW)) + buffer_check_shutw(s->rep); + if (!(s->req->flags & BF_SHUTR)) + buffer_check_shutr(s->req); + rqf_cli = s->req->flags; rpf_cli = s->rep->flags; } @@ -1175,7 +1180,10 @@ void process_session(struct task *t, int *next) buffer_shutw_now(s->req); } - stream_sock_data_update(s->req->cons->fd); + if (!(s->req->flags & BF_SHUTW)) + buffer_check_shutw(s->req); + if (!(s->rep->flags & BF_SHUTR)) + buffer_check_shutr(s->rep); /* When a server-side connection is released, we have to * count it and check for pending connections on this server. diff --git a/src/stream_sock.c b/src/stream_sock.c index a23d0fd321..f7b8d9d85a 100644 --- a/src/stream_sock.c +++ b/src/stream_sock.c @@ -471,105 +471,54 @@ int stream_sock_write(int fd) { /* * This function performs a shutdown-write on a stream interface in a connected or * init state (it does nothing for other states). It either shuts the write side - * closes the file descriptor and marks itself as closed. No buffer flags are + * or closes the file descriptor and marks itself as closed. No buffer flags are * changed, it's up to the caller to adjust them. The sole purpose of this * function is to be called from the other stream interface to notify of a * close_read, or by itself upon a full write leading to an empty buffer. - * It normally returns zero, unless it has completely closed the socket, in - * which case it returns 1. */ -int stream_sock_shutw(struct stream_interface *si) +void stream_sock_shutw(struct stream_interface *si) { - if (si->state != SI_ST_EST && si->state != SI_ST_CON) - return 0; + if (si->state != SI_ST_EST && si->state != SI_ST_CON) { + if (likely(si->state == SI_ST_INI)) + si->state = SI_ST_CLO; + return; + } if (si->ib->flags & BF_SHUTR) { fd_delete(si->fd); si->state = SI_ST_DIS; - return 1; + return; } EV_FD_CLR(si->fd, DIR_WR); shutdown(si->fd, SHUT_WR); - return 0; + return; } /* * This function performs a shutdown-read on a stream interface in a connected or - * init state (it does nothing for other states). It either shuts the read side or - * closes the file descriptor and marks itself as closed. No buffer flags are + * init state (it does nothing for other states). It either shuts the read side + * or closes the file descriptor and marks itself as closed. No buffer flags are * changed, it's up to the caller to adjust them. The sole purpose of this * function is to be called from the other stream interface to notify of a * close_read, or by itself upon a full write leading to an empty buffer. - * It normally returns zero, unless it has completely closed the socket, in - * which case it returns 1. */ -int stream_sock_shutr(struct stream_interface *si) +void stream_sock_shutr(struct stream_interface *si) { - if (si->state != SI_ST_EST && si->state != SI_ST_CON) - return 0; + if (si->state != SI_ST_EST && si->state != SI_ST_CON) { + if (likely(si->state == SI_ST_INI)) + si->state = SI_ST_CLO; + return; + } if (si->ob->flags & BF_SHUTW) { fd_delete(si->fd); si->state = SI_ST_DIS; - return 1; + return; } EV_FD_CLR(si->fd, DIR_RD); - return 0; -} - -/* - * Manages a stream_sock connection during its data phase. The buffers are - * examined for various cases of shutdown, then file descriptor and buffers' - * flags are updated accordingly. - */ -int stream_sock_data_update(int fd) -{ - struct buffer *ib = fdtab[fd].cb[DIR_RD].b; - struct buffer *ob = fdtab[fd].cb[DIR_WR].b; - - DPRINTF(stderr,"[%u] %s: fd=%d owner=%p ib=%p, ob=%p, exp(r,w)=%u,%u ibf=%08x obf=%08x ibl=%d obl=%d si=%d\n", - now_ms, __FUNCTION__, - fd, fdtab[fd].owner, - ib, ob, - ib->rex, ob->wex, - ib->flags, ob->flags, - ib->l, ob->l, ob->cons->state); - - /* Check if we need to close the read side */ - if (!(ib->flags & BF_SHUTR)) { - /* Last read, forced read-shutdown, or other end closed */ - if (ib->flags & (BF_SHUTR_NOW|BF_SHUTW)) { - //trace_term(t, TT_HTTP_SRV_10); - buffer_shutr(ib); - if (ob->flags & BF_SHUTW) { - fd_delete(fd); - ob->cons->state = SI_ST_DIS; - return 0; - } - EV_FD_CLR(fd, DIR_RD); - } - } - - /* Check if we need to close the write side */ - if (!(ob->flags & BF_SHUTW)) { - /* Forced write-shutdown or other end closed with empty buffer. */ - if ((ob->flags & BF_SHUTW_NOW) || - (ob->flags & (BF_EMPTY|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR)) == (BF_EMPTY|BF_WRITE_ENA|BF_SHUTR)) { - //trace_term(t, TT_HTTP_SRV_11); - buffer_shutw(ob); - if (ib->flags & BF_SHUTR) { - fd_delete(fd); - ob->cons->state = SI_ST_DIS; - return 0; - } - EV_FD_CLR(fd, DIR_WR); - shutdown(fd, SHUT_WR); - } - } - return 0; /* other cases change nothing */ + return; } - /* * Updates a connected stream_sock file descriptor status and timeouts * according to the buffers' flags. It should only be called once after the