[MAJOR] buffers: fix misuse of the BF_SHUTW_NOW flag
This flag was incorrectly used as meaning "close immediately",
while it needs to say "close ASAP". ASAP here means when unsent
data pending in the buffer are sent. This helps cleaning up some
dirty tricks where the buffer output was checking the BF_SHUTR
flag combined with EMPTY and other such things. Now we have a
clearly defined semantics :
- producer sets SHUTR and *may* set SHUTW_NOW if WRITE_ENA is
set, otherwise leave it to the session processor to set it.
- consumer only checks SHUTW_NOW to decide whether or not to
call shutw().
This also induced very minor changes at some locations which were
not protected against buffer changes while the SHUTW_NOW flag was
set. Now we prevent send_max from changing when the flag is set.
Several tests have been run without any unexpected behaviour detected.
Some more cleanups are needed, as it clearly appears that some tests
could be removed with stricter semantics.
diff --git a/src/session.c b/src/session.c
index 3e7484d..013be7f 100644
--- a/src/session.c
+++ b/src/session.c
@@ -198,9 +198,8 @@
}
/* OK, maybe we want to abort */
- if (unlikely((req->flags & BF_SHUTW_NOW) ||
- (rep->flags & BF_SHUTW) ||
- ((req->flags & BF_SHUTR) && /* FIXME: this should not prevent a connection from establishing */
+ 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) ||
s->be->options & PR_O_ABRT_CLOSE)))) {
/* give up */
@@ -451,8 +450,8 @@
}
/* Connection remains in queue, check if we have to abort it */
- if ((si->ob->flags & (BF_READ_ERROR|BF_SHUTW_NOW)) || /* abort requested */
- ((si->ob->flags & BF_SHUTR) && /* empty and client stopped */
+ 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))) {
/* give up */
si->exp = TICK_ETERNITY;
@@ -471,8 +470,8 @@
}
else if (si->state == SI_ST_TAR) {
/* Connection request might be aborted */
- if ((si->ob->flags & (BF_READ_ERROR|BF_SHUTW_NOW)) || /* abort requested */
- ((si->ob->flags & BF_SHUTR) && /* empty and client stopped */
+ 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))) {
/* give up */
si->exp = TICK_ETERNITY;
@@ -957,7 +956,7 @@
* of data to be forwarded from the producer to the consumer (which
* might possibly not be connected yet).
*/
- if (!(s->req->flags & BF_SHUTR) &&
+ if (!(s->req->flags & (BF_SHUTR|BF_SHUTW|BF_SHUTW_NOW)) &&
s->req->to_forward < FORWARD_DEFAULT_SIZE)
buffer_forward(s->req, FORWARD_DEFAULT_SIZE);
}
@@ -982,10 +981,10 @@
*/
/* first, let's check if the request buffer needs to shutdown(write) */
- if (unlikely((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_EMPTY|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR)) ==
- (BF_EMPTY|BF_WRITE_ENA|BF_SHUTR)))
+ if (unlikely((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR)) ==
+ (BF_WRITE_ENA|BF_SHUTR)))
buffer_shutw_now(s->req);
- else if ((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_EMPTY|BF_WRITE_ENA)) == (BF_EMPTY|BF_WRITE_ENA) &&
+ else if ((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_WRITE_ENA)) == (BF_WRITE_ENA) &&
(s->req->cons->state == SI_ST_EST) &&
s->be->options & PR_O_FORCE_CLO &&
s->rep->flags & BF_READ_ACTIVITY) {
@@ -997,7 +996,7 @@
}
/* shutdown(write) pending */
- if (unlikely((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW)) == BF_SHUTW_NOW))
+ if (unlikely((s->req->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_EMPTY)) == (BF_SHUTW_NOW|BF_EMPTY)))
s->req->cons->shutw(s->req->cons);
/* shutdown(write) done on server side, we must stop the client too */
@@ -1072,7 +1071,7 @@
* of data to be forwarded from the producer to the consumer (which
* might possibly not be connected yet).
*/
- if (!(s->rep->flags & BF_SHUTR) &&
+ if (!(s->rep->flags & (BF_SHUTR|BF_SHUTW|BF_SHUTW_NOW)) &&
s->rep->to_forward < FORWARD_DEFAULT_SIZE)
buffer_forward(s->rep, FORWARD_DEFAULT_SIZE);
}
@@ -1101,12 +1100,12 @@
*/
/* first, let's check if the response buffer needs to shutdown(write) */
- if (unlikely((s->rep->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_EMPTY|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR)) ==
- (BF_EMPTY|BF_WRITE_ENA|BF_SHUTR)))
+ if (unlikely((s->rep->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR)) ==
+ (BF_WRITE_ENA|BF_SHUTR)))
buffer_shutw_now(s->rep);
/* shutdown(write) pending */
- if (unlikely((s->rep->flags & (BF_SHUTW|BF_SHUTW_NOW)) == BF_SHUTW_NOW))
+ if (unlikely((s->rep->flags & (BF_SHUTW|BF_EMPTY|BF_SHUTW_NOW)) == (BF_EMPTY|BF_SHUTW_NOW)))
s->rep->cons->shutw(s->rep->cons);
/* shutdown(write) done on the client side, we must stop the server too */
diff --git a/src/stream_sock.c b/src/stream_sock.c
index b2bacdc..715259b 100644
--- a/src/stream_sock.c
+++ b/src/stream_sock.c
@@ -332,7 +332,7 @@
cur_read += ret;
/* if we're allowed to directly forward data, we must update send_max */
- if (b->to_forward > 0) {
+ if (b->to_forward > 0 && !(b->flags & (BF_SHUTW|BF_SHUTW_NOW))) {
int fwd = MIN(b->to_forward, ret);
b->send_max += fwd;
b->to_forward -= fwd;
@@ -502,6 +502,8 @@
/* we received a shutdown */
fdtab[fd].ev &= ~FD_POLL_HUP;
b->flags |= BF_READ_NULL;
+ if (b->flags & BF_WRITE_ENA)
+ buffer_shutw_now(b);
stream_sock_shutr(si);
goto out_wakeup;
@@ -599,7 +601,7 @@
unsigned int send_flag = MSG_DONTWAIT | MSG_NOSIGNAL;
if (MSG_MORE &&
- (((b->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR)) == (BF_WRITE_ENA|BF_SHUTR) &&
+ (((b->flags & (BF_SHUTW|BF_SHUTW_NOW|BF_HIJACK|BF_WRITE_ENA)) == (BF_WRITE_ENA|BF_SHUTW_NOW) &&
(max == b->l)) ||
(max != b->l && max != b->send_max))
&& (fdtab[si->fd].flags & FD_FL_TCP)) {
@@ -739,8 +741,8 @@
* send_max limit was reached. Maybe we just wrote the last
* chunk and need to close.
*/
- if (((b->flags & (BF_SHUTW|BF_EMPTY|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR)) ==
- (BF_EMPTY|BF_WRITE_ENA|BF_SHUTR)) &&
+ if (((b->flags & (BF_SHUTW|BF_HIJACK|BF_WRITE_ENA|BF_SHUTW_NOW)) ==
+ (BF_WRITE_ENA|BF_SHUTW_NOW)) &&
(si->state == SI_ST_EST)) {
stream_sock_shutw(si);
goto out_wakeup;
@@ -813,6 +815,7 @@
*/
void stream_sock_shutw(struct stream_interface *si)
{
+ si->ob->flags &= ~BF_SHUTW_NOW;
if (si->ob->flags & BF_SHUTW)
return;
si->ob->flags |= BF_SHUTW;
@@ -857,6 +860,7 @@
*/
void stream_sock_shutr(struct stream_interface *si)
{
+ si->ib->flags &= ~BF_SHUTR_NOW;
if (si->ib->flags & BF_SHUTR)
return;
si->ib->flags |= BF_SHUTR;
@@ -1041,8 +1045,8 @@
* send_max limit was reached. Maybe we just wrote the last
* chunk and need to close.
*/
- if (((ob->flags & (BF_SHUTW|BF_EMPTY|BF_HIJACK|BF_WRITE_ENA|BF_SHUTR)) ==
- (BF_EMPTY|BF_WRITE_ENA|BF_SHUTR)) &&
+ if (((ob->flags & (BF_SHUTW|BF_HIJACK|BF_WRITE_ENA|BF_SHUTW_NOW)) ==
+ (BF_WRITE_ENA|BF_SHUTW_NOW)) &&
(si->state == SI_ST_EST)) {
stream_sock_shutw(si);
goto out_wakeup;