BUG/MEDIUM: channel: incorrect polling condition may delay event delivery

The condition to poll for receive as implemented in channel_may_recv()
is still incorrect. If buf->o is null and buf->i is slightly larger than
chn->to_forward and at least as large as buf->size - maxrewrite, then
reading will be disabled. It may slightly delay some data delivery by
having first to forward pending bytes, but may also cause some random
issues with analysers that wait for some data before starting to forward
what they correctly parsed. For instance, a body analyser may be prevented
from seeing the data that only fits in the reserve.

This bug may also prevent an applet's chk_rcv() function from being called
when part of a buffer is released. It is possible (though not verified)
that this participated to some peers frozen session issues some people
have been facing.

This fix should be backported to 1.6 and 1.5 to ensure better coherency
with channel_recv_limit().

diff --git a/include/proto/channel.h b/include/proto/channel.h
index 76754e18ff6a8fe9339af85f361fac4d4ceab522..d2c8a150299dd437e3ed3384d4673cf86c6ed4a1 100644 (file)
--- a/include/proto/channel.h
+++ b/include/proto/channel.h
@@ -181,21 +181,21 @@ static inline int channel_may_recv(const struct channel *chn)
        if (!rem)
                return 0; /* buffer already full */
 
-       if (rem <= global.tune.maxrewrite && !channel_may_send(chn))
-               return 0;
-
-       /* now we know there's some room left, verify if we're touching
-        * the reserve with some permanent input data.
+       if (rem > global.tune.maxrewrite)
+               return 1; /* reserve not yet reached */
+
+       if (!channel_may_send(chn))
+               return 0; /* don't touch reserve until we can send */
+
+       /* Now we know there's some room left in the reserve and we may
+        * forward. As long as i-to_fwd < size-maxrw, we may still
+        * receive. This is equivalent to i+maxrw-size < to_fwd,
+        * which is logical since i+maxrw-size is what overlaps with
+        * the reserve, and we want to ensure they're covered by scheduled
+        * forwards.
         */
-       if (chn->to_forward >= chn->buf->i ||
-           (CHN_INFINITE_FORWARD < MAX_RANGE(typeof(chn->buf->i)) && // just there to ensure gcc
-            chn->to_forward == CHN_INFINITE_FORWARD))                // avoids the useless second
-               return 1;                                             // test whenever possible
-
-       rem -= global.tune.maxrewrite;
-       rem += chn->buf->o;
-       rem += chn->to_forward;
-       return rem > 0;
+       rem = chn->buf->i + global.tune.maxrewrite - chn->buf->size;
+       return rem < 0 || (unsigned int)rem < chn->to_forward;
 }
 
 /* Returns true if the channel's input is already closed */