mirror of
http://git.haproxy.org/git/haproxy.git/
synced 2025-02-22 13:46:52 +00:00
[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.
This commit is contained in:
parent
5af1fa1df0
commit
815a9b2039
@ -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;
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user