[BUG] session: analysers must be checked when SI state changes

Since the BF_READ_ATTACHED bug was fixed, a new issue surfaced. When
a connection closes on the return path in tunnel mode while the request
input is already closed, the request analyser which is waiting for a
state change never gets woken up so it never closes the request output.
This causes stuck sessions to remain indefinitely.

One way to reliably reproduce the issue is the following (note that the
client expects a keep-alive but not the server) :

  server: printf "HTTP/1.0 303\r\n\r\n" | nc -lp8080
  client: printf "GET / HTTP/1.1\r\n\r\n" | nc 127.1 2500

The reason for the issue is that we don't wake the analysers up on
stream interface state changes. So the least intrusive and most reliable
thing to do is to consider stream interface state changes to call the
analysers.

We just need to remember what state each series of analysers have seen
and check for the differences. In practice, that works.

A later improvement later could consist in being able to let analysers
state what they're interested to monitor :
  - left SI's state
  - right SI's state
  - request buffer flags
  - response buffer flags

That could help having only one set of analysers and call them once
status changes.

diff --git a/src/session.c b/src/session.c
index acd9770311d51f7e87ebc7255db68766645ad73c..84ffcbe4653846a2e757076cc324309c0eb2a3b4 100644 (file)
--- a/src/session.c
+++ b/src/session.c
@@ -1104,6 +1104,8 @@ struct task *process_session(struct task *t)
 {
        struct session *s = t->context;
        unsigned int rqf_last, rpf_last;
+       unsigned int rq_prod_last, rq_cons_last;
+       unsigned int rp_cons_last, rp_prod_last;
        unsigned int req_ana_back;
 
        //DPRINTF(stderr, "%s:%d: cs=%d ss=%d(%d) rqf=0x%08x rpf=0x%08x\n", __FUNCTION__, __LINE__,
@@ -1219,7 +1221,12 @@ struct task *process_session(struct task *t)
                 */
        }
 
-resync_stream_interface:
+       rq_prod_last = s->si[0].state;
+       rq_cons_last = s->si[1].state;
+       rp_cons_last = s->si[0].state;
+       rp_prod_last = s->si[1].state;
+
+ resync_stream_interface:
        /* Check for connection closure */
 
        DPRINTF(stderr,
@@ -1262,7 +1269,9 @@ resync_stream_interface:
  resync_request:
        /* Analyse request */
        if ((s->req->flags & BF_MASK_ANALYSER) ||
-           (s->req->flags ^ rqf_last) & BF_MASK_STATIC) {
+           ((s->req->flags ^ rqf_last) & BF_MASK_STATIC) ||
+           s->si[0].state != rq_prod_last ||
+           s->si[1].state != rq_cons_last) {
                unsigned int flags = s->req->flags;
 
                if (s->req->prod->state >= SI_ST_EST) {
@@ -1387,10 +1396,12 @@ resync_stream_interface:
                        }
                }
 
-               if ((s->req->flags ^ flags) & BF_MASK_STATIC) {
-                       rqf_last = s->req->flags;
+               rq_prod_last = s->si[0].state;
+               rq_cons_last = s->si[1].state;
+               rqf_last = s->req->flags;
+
+               if ((s->req->flags ^ flags) & BF_MASK_STATIC)
                        goto resync_request;
-               }
        }
 
        /* we'll monitor the request analysers while parsing the response,
@@ -1419,7 +1430,9 @@ resync_stream_interface:
                }
        }
        else if ((s->rep->flags & BF_MASK_ANALYSER) ||
-                (s->rep->flags ^ rpf_last) & BF_MASK_STATIC) {
+                (s->rep->flags ^ rpf_last) & BF_MASK_STATIC ||
+                s->si[0].state != rp_cons_last ||
+                s->si[1].state != rp_prod_last) {
                unsigned int flags = s->rep->flags;
 
                if (s->rep->prod->state >= SI_ST_EST) {
@@ -1478,10 +1491,12 @@ resync_stream_interface:
                        }
                }
 
-               if ((s->rep->flags ^ flags) & BF_MASK_STATIC) {
-                       rpf_last = s->rep->flags;
+               rp_cons_last = s->si[0].state;
+               rp_prod_last = s->si[1].state;
+               rpf_last = s->rep->flags;
+
+               if ((s->rep->flags ^ flags) & BF_MASK_STATIC)
                        goto resync_response;
-               }
        }
 
        /* maybe someone has added some request analysers, so we must check and loop */
@@ -1681,7 +1696,7 @@ resync_stream_interface:
        if (s->req->prod->state == SI_ST_DIS || s->req->cons->state == SI_ST_DIS)
                goto resync_stream_interface;
 
-       /* otherwise wewant to check if we need to resync the req buffer or not */
+       /* otherwise we want to check if we need to resync the req buffer or not */
        if ((s->req->flags ^ rqf_last) & BF_MASK_STATIC)
                goto resync_request;