BUG/MEDIUM: stconn: Only consider I/O timers to update stream's expiration date

In sc_notify(), it remained a case where it was possible to set an
expiration date on the stream in the past, leading to a crash because of a
BUG_ON(). This must never happen of course.

In sc_notify(), The stream's expiration may be updated in case no wakeup
conditions are encoutered. In that case, we must take care to never set an
expiration date in the past. However, it appeared there was still a
condition to do so. This code is based on an implicit postulate: the
stream's expiration date must always be set when we leave
process_stream(). It was true since the 2.9. But in 3.0, the buffer
allocation mechanism was improved and on an alloc failure in
process_stream(), the stream is inserted in a wait-list and its expiration
date is set to TICK_ETERNITY. With the good timing, and an analysis
expiration date set on a channel, it is possible to set the stream's
expiration date in past.

After analysis, it appeared that the proper way to fix the issue is to only
evaluate I/O timers (read and write timeout) and not stream's timers
(analase_exp or conn_exp) because only I/O timers may have changed since the
last process_stream() call.

This patch must be backported as far as 3.0 to fix the issue. But it is
probably a good idea to also backported it as far as 2.8.

(cherry picked from commit 4f32d03360e42a3e7fe4ed71012ec90339b9731d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 0510c41910778257fd398309045c9d09bf34ec49)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/stconn.c b/src/stconn.c
index 99947b4..f46be0b 100644 (file)
--- a/src/stconn.c
+++ b/src/stconn.c
@@ -1199,17 +1199,15 @@ void sc_notify(struct stconn *sc)
                task_wakeup(task, TASK_WOKEN_IO);
        }
        else {
-               /* Update expiration date for the task and requeue it if not already expired */
+               /* Update expiration date for the task and requeue it if not already expired.
+                * Only I/O timeouts are evaluated. The stream is responsible of others.
+                */
                if (!tick_is_expired(task->expire, now_ms)) {
                        task->expire = tick_first(task->expire, sc_ep_rcv_ex(sc));
                        task->expire = tick_first(task->expire, sc_ep_snd_ex(sc));
                        task->expire = tick_first(task->expire, sc_ep_rcv_ex(sco));
                        task->expire = tick_first(task->expire, sc_ep_snd_ex(sco));
-                       task->expire = tick_first(task->expire, ic->analyse_exp);
-                       task->expire = tick_first(task->expire, oc->analyse_exp);
-                       task->expire = tick_first(task->expire, __sc_strm(sc)->conn_exp);
 
-                       /* WARNING: Don't forget to remove this BUG_ON before 2.9.0 */
                        BUG_ON(tick_is_expired(task->expire, now_ms));
                        task_queue(task);
                }