]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: stconn: Report a blocked send if some output data are not consumed
authorChristopher Faulet <cfaulet@haproxy.com>
Mon, 27 Feb 2023 15:38:12 +0000 (16:38 +0100)
committerChristopher Faulet <cfaulet@haproxy.com>
Mon, 27 Feb 2023 16:45:45 +0000 (17:45 +0100)
Instead of reporting a blocked send if nothing is send, we do it if some
output data remain blocked after a write attempts or after a call the the
applet's I/O handler. It is mandatory to properly handle write timeouts.

Indeed, if an endpoint is blocked for a while but it partially consumed
output data, no timeout is triggered. It is especially true for
connections. But the same may happen for applet, there is no reason.

Of course, if the endpoint decides to partially consume output data because
it must wait to move on for any reason, it should use the se/applet API
(se/applet_will_consume(), se/applet_wont_consume() and
se/applet_need_more_data()).

This bug was introduced during the channels timeouts refactoring. No
backport is needed.

src/applet.c
src/stconn.c

index bfdab6f91d3eb0c528930955b255d5ae039e811a..2900013932a83d0188f1c8260325d4e4a5477705 100644 (file)
@@ -250,18 +250,16 @@ struct task *task_run_applet(struct task *t, void *context, unsigned int state)
        if (count != co_data(sc_oc(sc))) {
                sc_oc(sc)->flags |= CF_WRITE_EVENT | CF_WROTE_DATA;
                sc_have_room(sc_opposite(sc));
-               sc_ep_report_send_activity(sc);
-       }
-       else {
-               if (sc_ep_test(sc, SE_FL_WONT_CONSUME))
-                       sc_ep_report_send_activity(sc);
-               else
-                       sc_ep_report_blocked_send(sc);
        }
 
        if (sc_ic(sc)->flags & CF_READ_EVENT)
                sc_ep_report_read_activity(sc);
 
+       if (channel_is_empty(sc_oc(sc)))
+               sc_ep_report_send_activity(sc);
+       else
+               sc_ep_report_blocked_send(sc);
+
        /* measure the call rate and check for anomalies when too high */
        if (((b_size(sc_ib(sc)) && sc->flags & SC_FL_NEED_BUFF) || // asks for a buffer which is present
             (b_size(sc_ib(sc)) && !b_data(sc_ib(sc)) && sc->flags & SC_FL_NEED_ROOM) || // asks for room in an empty buffer
index 3c19cfc38c54cb6e1dce39bce158daa97a33fa20..39299b19a6d80d32f5a70bd769ad9085f4cf3862 100644 (file)
@@ -1667,12 +1667,8 @@ static int sc_conn_send(struct stconn *sc)
                oc->flags |= CF_WRITE_EVENT | CF_WROTE_DATA;
                if (sc->state == SC_ST_CON)
                        sc->state = SC_ST_RDY;
-
                sc_have_room(sc_opposite(sc));
-               sc_ep_report_send_activity(sc);
        }
-       else
-               sc_ep_report_blocked_send(sc);
 
        if (sc_ep_test(sc, SE_FL_ERROR | SE_FL_ERR_PENDING)) {
                oc->flags |= CF_WRITE_EVENT;
@@ -1681,9 +1677,14 @@ static int sc_conn_send(struct stconn *sc)
                return 1;
        }
 
-       /* We couldn't send all of our data, let the mux know we'd like to send more */
-       if (!channel_is_empty(oc))
+       if (channel_is_empty(oc))
+               sc_ep_report_send_activity(sc);
+       else {
+               /* We couldn't send all of our data, let the mux know we'd like to send more */
                conn->mux->subscribe(sc, SUB_RETRY_SEND, &sc->wait_event);
+               sc_ep_report_blocked_send(sc);
+       }
+
        return did_send;
 }