[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/
This commit is contained in:
Willy Tarreau 2008-11-23 19:31:35 +01:00
parent 74ab2ac7b0
commit 0a5d5ddeb9
6 changed files with 76 additions and 79 deletions

View File

@ -3,7 +3,7 @@
Buffer management definitions, macros and inline functions. Buffer management definitions, macros and inline functions.
Copyright (C) 2000-2008 Willy Tarreau - w@1wt.eu Copyright (C) 2000-2008 Willy Tarreau - w@1wt.eu
This library is free software; you can redistribute it and/or This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public modify it under the terms of the GNU Lesser General Public
License as published by the Free Software Foundation, version 2.1 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; 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 */ /* returns the maximum number of bytes writable at once in this buffer */
static inline int buffer_max(const struct buffer *buf) static inline int buffer_max(const struct buffer *buf)
{ {

View File

@ -33,10 +33,9 @@
/* main event functions used to move data between sockets and buffers */ /* main event functions used to move data between sockets and buffers */
int stream_sock_read(int fd); int stream_sock_read(int fd);
int stream_sock_write(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_data_finish(int fd);
int stream_sock_shutr(struct stream_interface *si); void stream_sock_shutr(struct stream_interface *si);
int stream_sock_shutw(struct stream_interface *si); void stream_sock_shutw(struct stream_interface *si);
/* This either returns the sockname or the original destination address. Code /* This either returns the sockname or the original destination address. Code

View File

@ -3,7 +3,7 @@
Buffer management definitions, macros and inline functions. Buffer management definitions, macros and inline functions.
Copyright (C) 2000-2008 Willy Tarreau - w@1wt.eu Copyright (C) 2000-2008 Willy Tarreau - w@1wt.eu
This library is free software; you can redistribute it and/or This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public modify it under the terms of the GNU Lesser General Public
License as published by the Free Software Foundation, version 2.1 License as published by the Free Software Foundation, version 2.1
@ -24,6 +24,7 @@
#include <common/config.h> #include <common/config.h>
#include <common/memory.h> #include <common/memory.h>
#include <types/stream_interface.h>
/* The BF_* macros designate Buffer Flags, which may be ORed in the bit field /* 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 : * member 'flags' in struct buffer. Here we have several types of flags :

View File

@ -74,8 +74,8 @@ struct stream_interface {
int fd; /* file descriptor for a stream driver when known */ int fd; /* file descriptor for a stream driver when known */
unsigned int flags; /* SI_FL_*, must be cleared before I/O */ unsigned int flags; /* SI_FL_*, must be cleared before I/O */
unsigned int exp; /* wake up time for connect, queue, turn-around, ... */ unsigned int exp; /* wake up time for connect, queue, turn-around, ... */
int (*shutr)(struct stream_interface *); /* shutr function */ void (*shutr)(struct stream_interface *); /* shutr function */
int (*shutw)(struct stream_interface *); /* shutw function */ void (*shutw)(struct stream_interface *); /* shutw function */
struct buffer *ib, *ob; /* input and output buffers */ struct buffer *ib, *ob; /* input and output buffers */
unsigned int err_type; /* first error detected, one of SI_ET_* */ unsigned int err_type; /* first error detected, one of SI_ET_* */
void *err_loc; /* commonly the server, NULL when SI_ET_NONE */ void *err_loc; /* commonly the server, NULL when SI_ET_NONE */

View File

@ -1136,7 +1136,12 @@ void process_session(struct task *t, int *next)
if (s->rep->cons->state != SI_ST_CLO) { if (s->rep->cons->state != SI_ST_CLO) {
if (((rqf_cli ^ s->req->flags) & BF_MASK_INTERFACE_I) || if (((rqf_cli ^ s->req->flags) & BF_MASK_INTERFACE_I) ||
((rpf_cli ^ s->rep->flags) & BF_MASK_INTERFACE_O)) { ((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; rqf_cli = s->req->flags;
rpf_cli = s->rep->flags; rpf_cli = s->rep->flags;
} }
@ -1175,7 +1180,10 @@ void process_session(struct task *t, int *next)
buffer_shutw_now(s->req); 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 /* When a server-side connection is released, we have to
* count it and check for pending connections on this server. * count it and check for pending connections on this server.

View File

@ -471,105 +471,54 @@ int stream_sock_write(int fd) {
/* /*
* This function performs a shutdown-write on a stream interface in a connected or * 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 * 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 * 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 * 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. * 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) if (si->state != SI_ST_EST && si->state != SI_ST_CON) {
return 0; if (likely(si->state == SI_ST_INI))
si->state = SI_ST_CLO;
return;
}
if (si->ib->flags & BF_SHUTR) { if (si->ib->flags & BF_SHUTR) {
fd_delete(si->fd); fd_delete(si->fd);
si->state = SI_ST_DIS; si->state = SI_ST_DIS;
return 1; return;
} }
EV_FD_CLR(si->fd, DIR_WR); EV_FD_CLR(si->fd, DIR_WR);
shutdown(si->fd, SHUT_WR); shutdown(si->fd, SHUT_WR);
return 0; return;
} }
/* /*
* This function performs a shutdown-read on a stream interface in a connected or * 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 * init state (it does nothing for other states). It either shuts the read 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 * 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 * 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. * 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) if (si->state != SI_ST_EST && si->state != SI_ST_CON) {
return 0; if (likely(si->state == SI_ST_INI))
si->state = SI_ST_CLO;
return;
}
if (si->ob->flags & BF_SHUTW) { if (si->ob->flags & BF_SHUTW) {
fd_delete(si->fd); fd_delete(si->fd);
si->state = SI_ST_DIS; si->state = SI_ST_DIS;
return 1; return;
} }
EV_FD_CLR(si->fd, DIR_RD); EV_FD_CLR(si->fd, DIR_RD);
return 0; return;
} }
/*
* 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 */
}
/* /*
* Updates a connected stream_sock file descriptor status and timeouts * Updates a connected stream_sock file descriptor status and timeouts
* according to the buffers' flags. It should only be called once after the * according to the buffers' flags. It should only be called once after the