BUG/MAJOR: session: revert all the crappy client-side timeout changes
This is the 3rd regression caused by the changes below. The latest to
date was reported by Finn Arne Gangstad. If a server responds with no
content-length and the client's FIN is never received, either we leak
the client-side FD or we spin at 100% CPU if timeout client-fin is set.
Enough is enough. The amount of tricks needed to cover these side-effects
starts to look like used toilet paper stacked over a chocolate cake. I
don't want to eat that cake anymore!
All this to avoid reporting a server-side timeout when a client stops
uploading data and haproxy expires faster than the server... A lot of
"ifs" resulting in a technically valid log that doesn't always please
users, and whose alternative causes that many issues for all others
users.
So let's revert this crap merged since 1.5-dev25 :
Revert "CLEANUP: http: don't clear CF_READ_NOEXP twice"
This reverts commit 1592d1e72a4a2d25a554c299ae95a3e6cad80bf1.
Revert "BUG/MEDIUM: http: clear CF_READ_NOEXP when preparing a new transaction"
This reverts commit 77d29029af1c44216b190dd7442964b9d8f45257.
Revert "BUG/MEDIUM: session: don't clear CF_READ_NOEXP if analysers are not called"
This reverts commit 0943757a2144761c60e416b5ed07baa76934f5a4.
Revert "BUG/MEDIUM: http: disable server-side expiration until client has sent the body"
This reverts commit 3bed5e9337fd6eeab0f0006ebefcbe98ee5c4f9f.
Revert "BUG/MEDIUM: http: correctly report request body timeouts"
This reverts commit b9edf8fbecc9d1b5c82794735adcc367a80a4ae2.
Revert "BUG/MEDIUM: http/session: disable client-side expiration only after body"
This reverts commit b1982e27aaff2a92a389a9f1bc847e3bb8fdb4f2.
If a cleaner AND SAFER way to do something equivalent in 1.6-dev, we *might*
consider backporting it to 1.5, but given the vicious bugs that have surfaced
since, I doubt it will happen any time soon.
Fortunately, that crap never made it into 1.4 so no backport is needed.
(cherry picked from commit 6f0a7bac282c9b2082dc763977b7721b6b002089)
diff --git a/src/session.c b/src/session.c
index f828d9c..e26f5ad 100644
--- a/src/session.c
+++ b/src/session.c
@@ -1636,7 +1636,6 @@
unsigned int rq_prod_last, rq_cons_last;
unsigned int rp_cons_last, rp_prod_last;
unsigned int req_ana_back;
- unsigned int rq_oneshot, rp_oneshot;
//DPRINTF(stderr, "%s:%d: cs=%d ss=%d(%d) rqf=0x%08x rpf=0x%08x\n", __FUNCTION__, __LINE__,
// s->si[0].state, s->si[1].state, s->si[1].err_type, s->req->flags, s->rep->flags);
@@ -1644,13 +1643,9 @@
/* this data may be no longer valid, clear it */
memset(&s->txn.auth, 0, sizeof(s->txn.auth));
- /* These flags must explicitly be set every time by the analysers who
- * need them, but we won't always call them (eg: during a connection
- * retry). So we need to keep them and only clear them if we're sure
- * to call the analysers.
- */
- rq_oneshot = s->req->flags & (CF_READ_NOEXP | CF_WAKE_WRITE);
- rp_oneshot = s->rep->flags & (CF_READ_NOEXP | CF_WAKE_WRITE);
+ /* This flag must explicitly be set every time */
+ s->req->flags &= ~(CF_READ_NOEXP|CF_WAKE_WRITE);
+ s->rep->flags &= ~(CF_READ_NOEXP|CF_WAKE_WRITE);
/* Keep a copy of req/rep flags so that we can detect shutdowns */
rqf_last = s->req->flags & ~CF_MASK_ANALYSER;
@@ -1831,8 +1826,6 @@
s->si[1].state != rq_cons_last) {
unsigned int flags = s->req->flags;
- s->req->flags &= ~rq_oneshot;
- rq_oneshot = 0;
if (s->req->prod->state >= SI_ST_EST) {
int max_loops = global.tune.maxpollevents;
unsigned int ana_list;
@@ -1986,13 +1979,11 @@
/* Analyse response */
if (((s->rep->flags & ~rpf_last) & CF_MASK_ANALYSER) ||
- ((s->rep->flags ^ rpf_last) & CF_MASK_STATIC) ||
- s->si[0].state != rp_cons_last ||
- s->si[1].state != rp_prod_last) {
+ (s->rep->flags ^ rpf_last) & CF_MASK_STATIC ||
+ s->si[0].state != rp_cons_last ||
+ s->si[1].state != rp_prod_last) {
unsigned int flags = s->rep->flags;
- s->rep->flags &= ~rp_oneshot;
- rp_oneshot = 0;
if ((s->rep->flags & CF_MASK_ANALYSER) &&
(s->rep->analysers & AN_REQ_WAIT_HTTP)) {
/* Due to HTTP pipelining, the HTTP request analyser might be waiting
@@ -2186,9 +2177,6 @@
channel_auto_close(s->req);
buffer_flush(s->req->buf);
- s->req->flags &= ~rq_oneshot;
- rq_oneshot = 0;
-
/* We'll let data flow between the producer (if still connected)
* to the consumer (which might possibly not be connected yet).
*/
@@ -2344,9 +2332,6 @@
channel_auto_close(s->rep);
buffer_flush(s->rep->buf);
- s->rep->flags &= ~rp_oneshot;
- rp_oneshot = 0;
-
/* We'll let data flow between the producer (if still connected)
* to the consumer.
*/
@@ -2496,6 +2481,20 @@
s->si[0].flags &= ~(SI_FL_ERR|SI_FL_EXP);
s->si[1].flags &= ~(SI_FL_ERR|SI_FL_EXP);
+ /* Trick: if a request is being waiting for the server to respond,
+ * and if we know the server can timeout, we don't want the timeout
+ * to expire on the client side first, but we're still interested
+ * in passing data from the client to the server (eg: POST). Thus,
+ * we can cancel the client's request timeout if the server's
+ * request timeout is set and the server has not yet sent a response.
+ */
+
+ if ((s->rep->flags & (CF_AUTO_CLOSE|CF_SHUTR)) == 0 &&
+ (tick_isset(s->req->wex) || tick_isset(s->rep->rex))) {
+ s->req->flags |= CF_READ_NOEXP;
+ s->req->rex = TICK_ETERNITY;
+ }
+
/* When any of the stream interfaces is attached to an applet,
* we have to call it here. Note that this one may wake the
* task up again. If at least one applet was called, the current