mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-22 22:31:28 +02:00
Code Issues Projects Releases Wiki Activity

[MAJOR] buffers: fix the BF_EMPTY flag's meaning

Browse Source 
The BF_EMPTY flag was once used to indicate an empty buffer. However,
it was used half the time as meaning the buffer is empty for the reader,
and half the time as meaning there is nothing left to send.

"nothing to send" is only indicated by "->send_max=0 && !pipe". Once
we fix this, we discover that the flag is not used anymore. So the
flags has been renamed BF_OUT_EMPTY and means exactly the condition
above, ie, there is nothing to send.

Doing so has allowed us to remove some unused tests for emptiness,
but also to uncover a certain amount of situations where the flag
was not correctly set or tested.
This commit is contained in:
Willy Tarreau 2009-09-20 08:09:44 +02:00
parent 520d95e42b
commit ba0b63d2c7
5 changed files with 60 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
include/proto/buffers.h
View File

@ -42,7 +42,7 @@ int init_buffer();
* so that the compiler can optimize it away if changed immediately after the
* call to this function. By default, it is set to the full size of the buffer.
* This implies that buffer_init() must only be called once ->size is set !
* The BF_EMPTY flags is set.
* The BF_OUT_EMPTY flags is set.
*/
static inline void buffer_init(struct buffer *buf)
{
@ -52,22 +52,11 @@ static inline void buffer_init(struct buffer *buf)
buf->pipe = NULL;
buf->analysers = 0;
buf->cons = NULL;
buf->flags = BF_EMPTY;
buf->flags = BF_OUT_EMPTY;
buf->r = buf->lr = buf->w = buf->data;
buf->max_len = buf->size;
}
/* returns 1 if the buffer is empty, 0 otherwise */
static inline int buffer_isempty(const struct buffer *buf)
{
return buf->l == 0;
}
/* returns 1 if the buffer is full, 0 otherwise */
static inline int buffer_isfull(const struct buffer *buf) {
return buf->l == buf->size;
}
/* Check buffer timeouts, and set the corresponding flags. The
* likely/unlikely have been optimized for fastest normal path.
* The read/write timeouts are not set if there was activity on the buffer.
@ -100,6 +89,9 @@ static inline void buffer_forward(struct buffer *buf, unsigned int bytes)
{
unsigned int data_left;
if (!bytes)
return;
buf->flags &= ~BF_OUT_EMPTY;
data_left = buf->l - buf->send_max;
if (data_left >= bytes) {
buf->send_max += bytes;
@ -118,6 +110,8 @@ static inline void buffer_flush(struct buffer *buf)
{
if (buf->send_max < buf->l)
buf->send_max = buf->l;
if (buf->send_max)
buf->flags &= ~BF_OUT_EMPTY;
}
/* Erase any content from buffer <buf> and adjusts flags accordingly. Note
@ -130,9 +124,11 @@ static inline void buffer_erase(struct buffer *buf)
buf->to_forward = 0;
buf->r = buf->lr = buf->w = buf->data;
buf->l = 0;
buf->flags |= BF_EMPTY | BF_FULL;
if (buf->max_len)
buf->flags &= ~BF_FULL;
buf->flags &= ~(BF_FULL | BF_OUT_EMPTY);
if (!buf->pipe)
buf->flags |= BF_OUT_EMPTY;
if (!buf->max_len)
buf->flags |= BF_FULL;
}
/* Cut the "tail" of the buffer, which means strip it to the length of unsent
@ -154,7 +150,7 @@ static inline void buffer_cut_tail(struct buffer *buf)
if (buf->r >= buf->data + buf->size)
buf->r -= buf->size;
buf->lr = buf->r;
buf->flags &= ~(BF_EMPTY|BF_FULL);
buf->flags &= ~BF_FULL;
if (buf->l >= buf->max_len)
buf->flags |= BF_FULL;
}
@ -336,16 +332,15 @@ static inline void buffer_skip(struct buffer *buf, int len)
buf->w = buf->data; /* wrap around the buffer */
buf->l -= len;
if (!buf->l) {
if (!buf->l)
buf->r = buf->w = buf->lr = buf->data;
if (!buf->pipe)
buf->flags |= BF_EMPTY;
}
if (buf->l < buf->max_len)
buf->flags &= ~BF_FULL;
buf->send_max -= len;
if (!buf->send_max && !buf->pipe)
buf->flags |= BF_OUT_EMPTY;
}
/*
@ -375,11 +370,6 @@ static inline int buffer_si_putchar(struct buffer *buf, char c)
if (buf->flags & BF_FULL)
return 0;
if (buf->flags & BF_EMPTY) {
buf->flags &= ~BF_EMPTY;
buf->r = buf->w = buf->lr = buf->data;
}
*buf->r = c;
buf->l++;
@ -393,6 +383,7 @@ static inline int buffer_si_putchar(struct buffer *buf, char c)
if ((signed)(buf->to_forward - 1) >= 0) {
buf->to_forward--;
buf->send_max++;
buf->flags &= ~BF_OUT_EMPTY;
}
buf->total++;

4
include/types/buffers.h
View File

@ -68,7 +68,7 @@
#define BF_WRITE_ERROR 0x000800 /* unrecoverable error on consumer side */
#define BF_WRITE_ACTIVITY (BF_WRITE_NULL|BF_WRITE_PARTIAL|BF_WRITE_ERROR)
#define BF_EMPTY 0x001000 /* buffer is empty */
#define BF_OUT_EMPTY 0x001000 /* send_max and pipe are empty. Set by last change. */
#define BF_SHUTW 0x002000 /* consumer has already shut down */
#define BF_SHUTW_NOW 0x004000 /* the consumer must shut down for writes ASAP */
#define BF_AUTO_CLOSE 0x008000 /* producer can forward shutdown to other side */
@ -120,7 +120,7 @@
#define BF_MASK_ANALYSER (BF_READ_ATTACHED|BF_READ_ACTIVITY|BF_READ_TIMEOUT|BF_ANA_TIMEOUT|BF_WRITE_ACTIVITY)
/* Mask for static flags which are not events, but might change during processing */
#define BF_MASK_STATIC (BF_EMPTY|BF_FULL|BF_HIJACK|BF_AUTO_CLOSE|BF_AUTO_CONNECT|BF_SHUTR|BF_SHUTW|BF_SHUTR_NOW|BF_SHUTW_NOW)
#define BF_MASK_STATIC (BF_OUT_EMPTY|BF_FULL|BF_HIJACK|BF_AUTO_CLOSE|BF_AUTO_CONNECT|BF_SHUTR|BF_SHUTW|BF_SHUTR_NOW|BF_SHUTW_NOW)
/* Analysers (buffer->analysers).

21
src/buffers.c
View File

@ -64,9 +64,7 @@ int buffer_write(struct buffer *buf, const char *msg, int len)
if (buf->r == buf->data + buf->size)
buf->r = buf->data;
buf->flags &= ~(BF_EMPTY|BF_FULL);
if (buf->l == 0)
buf->flags |= BF_EMPTY;
buf->flags &= ~(BF_OUT_EMPTY|BF_FULL);
if (buf->l >= buf->max_len)
buf->flags |= BF_FULL;
@ -107,12 +105,13 @@ int buffer_feed(struct buffer *buf, const char *str, int len)
int fwd = MIN(buf->to_forward, len);
buf->send_max += fwd;
buf->to_forward -= fwd;
buf->flags &= ~BF_OUT_EMPTY;
}
if (buf->r == buf->data + buf->size)
buf->r = buf->data;
buf->flags &= ~(BF_EMPTY|BF_FULL);
buf->flags &= ~BF_FULL;
if (buf->l >= buf->max_len)
buf->flags |= BF_FULL;
@ -174,6 +173,8 @@ int buffer_si_peekline(struct buffer *buf, char *str, int len)
* <b>'s parameters (l, r, w, h, lr) are recomputed to be valid after the shift.
* the shift value (positive or negative) is returned.
* If there's no space left, the move is not done.
* The function does not adjust ->send_max nor BF_OUT_EMPTY because it does not
* make sense to use it on data scheduled to be sent.
*
*/
int buffer_replace(struct buffer *b, char *pos, char *end, const char *str)
@ -199,9 +200,9 @@ int buffer_replace(struct buffer *b, char *pos, char *end, const char *str)
if (b->lr > pos) b->lr += delta;
b->l += delta;
b->flags &= ~(BF_EMPTY|BF_FULL);
b->flags &= ~BF_FULL;
if (b->l == 0)
b->flags |= BF_EMPTY;
b->r = b->w = b->lr = b->data;
if (b->l >= b->max_len)
b->flags |= BF_FULL;
@ -240,9 +241,9 @@ int buffer_replace2(struct buffer *b, char *pos, char *end, const char *str, int
if (b->lr > pos) b->lr += delta;
b->l += delta;
b->flags &= ~(BF_EMPTY|BF_FULL);
b->flags &= ~BF_FULL;
if (b->l == 0)
b->flags |= BF_EMPTY;
b->r = b->w = b->lr = b->data;
if (b->l >= b->max_len)
b->flags |= BF_FULL;
@ -284,9 +285,7 @@ int buffer_insert_line2(struct buffer *b, char *pos, const char *str, int len)
if (b->lr > pos) b->lr += delta;
b->l += delta;
b->flags &= ~(BF_EMPTY|BF_FULL);
if (b->l == 0)
b->flags |= BF_EMPTY;
b->flags &= ~BF_FULL;
if (b->l >= b->max_len)
b->flags |= BF_FULL;

15
src/session.c
View File

@ -200,7 +200,7 @@ int sess_update_st_con_tcp(struct session *s, struct stream_interface *si)
/* OK, maybe we want to abort */
if (unlikely((rep->flags & BF_SHUTW) ||
((req->flags & BF_SHUTW_NOW) && /* FIXME: this should not prevent a connection from establishing */
(((req->flags & (BF_EMPTY|BF_WRITE_ACTIVITY)) == BF_EMPTY) ||
(((req->flags & (BF_OUT_EMPTY|BF_WRITE_ACTIVITY)) == BF_OUT_EMPTY) ||
s->be->options & PR_O_ABRT_CLOSE)))) {
/* give up */
si->shutw(si);
@ -452,7 +452,7 @@ void sess_update_stream_int(struct session *s, struct stream_interface *si)
/* Connection remains in queue, check if we have to abort it */
if ((si->ob->flags & (BF_READ_ERROR)) ||
((si->ob->flags & BF_SHUTW_NOW) && /* empty and client aborted */
(si->ob->flags & BF_EMPTY || s->be->options & PR_O_ABRT_CLOSE))) {
(si->ob->flags & BF_OUT_EMPTY || s->be->options & PR_O_ABRT_CLOSE))) {
/* give up */
si->exp = TICK_ETERNITY;
s->logs.t_queue = tv_ms_elapsed(&s->logs.tv_accept, &now);
@ -472,7 +472,7 @@ void sess_update_stream_int(struct session *s, struct stream_interface *si)
/* Connection request might be aborted */
if ((si->ob->flags & (BF_READ_ERROR)) ||
((si->ob->flags & BF_SHUTW_NOW) && /* empty and client aborted */
(si->ob->flags & BF_EMPTY || s->be->options & PR_O_ABRT_CLOSE))) {
(si->ob->flags & BF_OUT_EMPTY || s->be->options & PR_O_ABRT_CLOSE))) {
/* give up */
si->exp = TICK_ETERNITY;
si->shutr(si);
@ -992,11 +992,11 @@ resync_stream_interface:
((s->req->cons->state == SI_ST_EST) &&
(s->be->options & PR_O_FORCE_CLO) &&
(s->rep->flags & BF_READ_ACTIVITY))))
buffer_shutw_now(s->req);
buffer_shutw_now(s->req);
}
/* shutdown(write) pending */
if (unlikely((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_EMPTY)) == (BF_SHUTW_NOW|BF_EMPTY)))
if (unlikely((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_OUT_EMPTY)) == (BF_SHUTW_NOW|BF_OUT_EMPTY)))
s->req->cons->shutw(s->req->cons);
/* shutdown(write) done on server side, we must stop the client too */
@ -1015,8 +1015,7 @@ resync_stream_interface:
*/
if (s->req->cons->state == SI_ST_INI) {
if (!(s->req->flags & (BF_SHUTW|BF_SHUTW_NOW))) {
if ((s->req->flags & BF_AUTO_CONNECT) ||
(s->req->send_max || s->req->pipe)) {
if ((s->req->flags & (BF_AUTO_CONNECT|BF_OUT_EMPTY)) != BF_OUT_EMPTY) {
/* If we have a ->connect method, we need to perform a connection request,
* otherwise we immediately switch to the connected state.
*/
@ -1112,7 +1111,7 @@ resync_stream_interface:
buffer_shutw_now(s->rep);
/* shutdown(write) pending */
if (unlikely((s->rep->flags & (BF_SHUTW|BF_EMPTY|BF_SHUTW_NOW)) == (BF_EMPTY|BF_SHUTW_NOW)))
if (unlikely((s->rep->flags & (BF_SHUTW|BF_OUT_EMPTY|BF_SHUTW_NOW)) == (BF_OUT_EMPTY|BF_SHUTW_NOW)))
s->rep->cons->shutw(s->rep->cons);
/* shutdown(write) done on the client side, we must stop the server too */

52
src/stream_sock.c
View File

@ -94,7 +94,7 @@ _syscall6(int, splice, int, fdin, loff_t *, off_in, int, fdout, loff_t *, off_ou
* BF_READ_NULL
* BF_READ_PARTIAL
* BF_WRITE_PARTIAL (during copy)
* BF_EMPTY (during copy)
* BF_OUT_EMPTY (during copy)
* SI_FL_ERR
* SI_FL_WAIT_ROOM
* (SI_FL_WAIT_RECV)
@ -207,7 +207,7 @@ static int stream_sock_splice_in(struct buffer *b, struct stream_interface *si)
b->total += ret;
b->pipe->data += ret;
b->flags |= BF_READ_PARTIAL;
b->flags &= ~BF_EMPTY; /* to prevent shutdowns */
b->flags &= ~BF_OUT_EMPTY;
if (b->pipe->data >= SPLICE_FULL_HINT ||
ret >= global.tune.recv_enough) {
@ -336,13 +336,13 @@ int stream_sock_read(int fd) {
int fwd = MIN(b->to_forward, ret);
b->send_max += fwd;
b->to_forward -= fwd;
b->flags &= ~BF_OUT_EMPTY;
}
if (fdtab[fd].state == FD_STCONN)
fdtab[fd].state = FD_STREADY;
b->flags |= BF_READ_PARTIAL;
b->flags &= ~BF_EMPTY;
if (b->r == b->data + b->size) {
b->r = b->data; /* wrap around the buffer */
@ -466,7 +466,7 @@ int stream_sock_read(int fd) {
out_wakeup:
/* We might have some data the consumer is waiting for */
if ((b->send_max || b->pipe) && (b->cons->flags & SI_FL_WAIT_DATA)) {
if (!(b->flags & BF_OUT_EMPTY) && (b->cons->flags & SI_FL_WAIT_DATA)) {
int last_len = b->pipe ? b->pipe->data : 0;
b->cons->chk_snd(b->cons);
@ -566,13 +566,11 @@ static int stream_sock_write_loop(struct stream_interface *si, struct buffer *b)
/* At this point, the pipe is empty, but we may still have data pending
* in the normal buffer.
*/
if (!b->l) {
b->flags |= BF_EMPTY;
#endif
if (!b->send_max) {
b->flags |= BF_OUT_EMPTY;
return retval;
}
#endif
if (!b->send_max)
return retval;
/* when we're in this loop, we already know that there is no spliced
* data left, and that there are sendable buffered data.
@ -634,16 +632,16 @@ static int stream_sock_write_loop(struct stream_interface *si, struct buffer *b)
if (likely(b->l < b->max_len))
b->flags &= ~BF_FULL;
if (likely(!b->l)) {
if (likely(!b->l))
/* optimize data alignment in the buffer */
b->r = b->w = b->lr = b->data;
if (likely(!b->pipe))
b->flags |= BF_EMPTY;
}
b->send_max -= ret;
if (!b->send_max || !b->l)
if (!b->send_max) {
if (likely(!b->pipe))
b->flags |= BF_OUT_EMPTY;
break;
}
/* if the system buffer is full, don't insist */
if (ret < max)
@ -691,7 +689,7 @@ int stream_sock_write(int fd)
if (b->flags & BF_SHUTW)
goto out_wakeup;
if (likely(!(b->flags & BF_EMPTY))) {
if (likely(!(b->flags & BF_OUT_EMPTY))) {
/* OK there are data waiting to be sent */
retval = stream_sock_write_loop(si, b);
if (retval < 0)
@ -735,7 +733,7 @@ int stream_sock_write(int fd)
*/
}
if (!b->pipe && !b->send_max) {
if (b->flags & BF_OUT_EMPTY) {
/* the connection is established but we can't write. Either the
* buffer is empty, or we just refrain from sending because the
* send_max limit was reached. Maybe we just wrote the last
@ -747,7 +745,7 @@ int stream_sock_write(int fd)
goto out_wakeup;
}
if ((b->flags & (BF_EMPTY|BF_SHUTW)) == BF_EMPTY)
if ((b->flags & (BF_SHUTW|BF_FULL|BF_HIJACK)) == 0)
si->flags |= SI_FL_WAIT_DATA;
EV_FD_CLR(fd, DIR_WR);
@ -757,8 +755,7 @@ int stream_sock_write(int fd)
out_may_wakeup:
if (b->flags & BF_WRITE_ACTIVITY) {
/* update timeout if we have written something */
if ((b->send_max || b->pipe) &&
(b->flags & (BF_SHUTW|BF_WRITE_PARTIAL)) == BF_WRITE_PARTIAL)
if ((b->flags & (BF_OUT_EMPTY|BF_SHUTW|BF_WRITE_PARTIAL)) == BF_WRITE_PARTIAL)
b->wex = tick_add_ifset(now_ms, b->wto);
out_wakeup:
@ -781,7 +778,7 @@ int stream_sock_write(int fd)
* any more data to forward and it's not planned to send any more.
*/
if (likely((b->flags & (BF_WRITE_NULL|BF_WRITE_ERROR|BF_SHUTW)) ||
(!b->to_forward && !b->send_max && !b->pipe) ||
((b->flags & BF_OUT_EMPTY) && !b->to_forward) ||
si->state != SI_ST_EST ||
b->prod->state != SI_ST_EST))
task_wakeup(si->owner, TASK_WOKEN_IO);
@ -925,9 +922,9 @@ void stream_sock_data_finish(struct stream_interface *si)
/* Check if we need to close the write side */
if (!(ob->flags & BF_SHUTW)) {
/* Write not closed, update FD status and timeout for writes */
if ((ob->send_max == 0 && !ob->pipe) || (ob->flags & BF_EMPTY)) {
if (ob->flags & BF_OUT_EMPTY) {
/* stop writing */
if ((ob->flags & (BF_EMPTY|BF_HIJACK)) == BF_EMPTY)
if ((ob->flags & (BF_FULL|BF_HIJACK)) == 0)
si->flags |= SI_FL_WAIT_DATA;
EV_FD_COND_C(fd, DIR_WR);
ob->wex = TICK_ETERNITY;
@ -1011,7 +1008,7 @@ void stream_sock_chk_snd(struct stream_interface *si)
if (!(si->flags & SI_FL_WAIT_DATA) || /* not waiting for data */
(fdtab[si->fd].ev & FD_POLL_OUT) || /* we'll be called anyway */
!(ob->send_max || ob->pipe)) /* called with nothing to send ! */
(ob->flags & BF_OUT_EMPTY)) /* called with nothing to send ! */
return;
retval = stream_sock_write_loop(si, ob);
@ -1035,7 +1032,7 @@ void stream_sock_chk_snd(struct stream_interface *si)
* been sent, and that we may have to poll first. We have to do that
* too if the buffer is not empty.
*/
if (ob->send_max == 0 && !ob->pipe) {
if (ob->flags & BF_OUT_EMPTY) {
/* the connection is established but we can't write. Either the
* buffer is empty, or we just refrain from sending because the
* send_max limit was reached. Maybe we just wrote the last
@ -1048,7 +1045,7 @@ void stream_sock_chk_snd(struct stream_interface *si)
goto out_wakeup;
}
if ((ob->flags & (BF_SHUTW|BF_EMPTY|BF_HIJACK)) == BF_EMPTY)
if ((ob->flags & (BF_SHUTW|BF_FULL|BF_HIJACK)) == 0)
si->flags |= SI_FL_WAIT_DATA;
ob->wex = TICK_ETERNITY;
}
@ -1064,8 +1061,7 @@ void stream_sock_chk_snd(struct stream_interface *si)
if (likely(ob->flags & BF_WRITE_ACTIVITY)) {
/* update timeout if we have written something */
if ((ob->send_max || ob->pipe) &&
(ob->flags & (BF_SHUTW|BF_WRITE_PARTIAL)) == BF_WRITE_PARTIAL)
if ((ob->flags & (BF_OUT_EMPTY|BF_SHUTW|BF_WRITE_PARTIAL)) == BF_WRITE_PARTIAL)
ob->wex = tick_add_ifset(now_ms, ob->wto);
if (tick_isset(si->ib->rex)) {
@ -1083,7 +1079,7 @@ void stream_sock_chk_snd(struct stream_interface *si)
* have to notify the task.
*/
if (likely((ob->flags & (BF_WRITE_NULL|BF_WRITE_ERROR|BF_SHUTW)) ||
(!ob->to_forward && !ob->send_max && !ob->pipe) ||
((ob->flags & BF_OUT_EMPTY) && !ob->to_forward) ||
si->state != SI_ST_EST)) {
out_wakeup:
task_wakeup(si->owner, TASK_WOKEN_IO);