MEDIUM: mux-fcgi: Introduce flags to deal with connection read/write errors

Similarly to the H1 and H2 multiplexers, FCFI_CF_ERR_PENDING is now used to
report an error when we try to send data and FCGI_CF_ERROR to report an
error when we try to read data. In other funcions, we rely on these flags
instead of connection ones. Only FCGI_CF_ERROR is considered as a final
error.  FCGI_CF_ERR_PENDING does not block receive attempt.

In addition, FCGI_CF_EOS flag was added. we rely on it to test if a read0
was received or not.

diff --git a/include/haproxy/mux_fcgi-t.h b/include/haproxy/mux_fcgi-t.h
index a321f62f53cf5d6e1c1ecb384b92727cb936150a..4599a36da5a354bd112a20fcaa3717344bca215b 100644 (file)
--- a/include/haproxy/mux_fcgi-t.h
+++ b/include/haproxy/mux_fcgi-t.h
@@ -54,6 +54,11 @@
 #define FCGI_CF_KEEP_CONN       0x00001000  /* HAProxy is responsible to close the connection */
 #define FCGI_CF_GET_VALUES      0x00002000  /* retrieve settings */
 
+#define FCGI_CF_EOS             0x00004000  /* End-of-stream seen on the H1 connection (read0 detected) */
+#define FCGI_CF_ERR_PENDING     0x00008000  /* A write error was detected (block sends but not reads) */
+#define FCGI_CF_ERROR           0x00010000  /* A read error was detected (handled has an abort) */
+
+
 /* This function is used to report flags in debugging tools. Please reflect
  * below any single-bit flag addition above in the same order via the
  * __APPEND_FLAG macro. The new end of the buffer is returned.
@@ -68,7 +73,8 @@ static forceinline char *fconn_show_flags(char *buf, size_t len, const char *del
        _(FCGI_CF_DEM_DALLOC, _(FCGI_CF_DEM_DFULL, _(FCGI_CF_DEM_MROOM,
        _(FCGI_CF_DEM_SALLOC, _(FCGI_CF_DEM_SFULL, _(FCGI_CF_DEM_TOOMANY,
        _(FCGI_CF_MPXS_CONNS, _(FCGI_CF_ABRTS_SENT, _(FCGI_CF_ABRTS_FAILED,
-       _(FCGI_CF_WAIT_FOR_HS, _(FCGI_CF_KEEP_CONN, _(FCGI_CF_GET_VALUES))))))))))))));
+       _(FCGI_CF_WAIT_FOR_HS, _(FCGI_CF_KEEP_CONN, _(FCGI_CF_GET_VALUES,
+       _(FCGI_CF_EOS, _(FCGI_CF_ERR_PENDING, _(FCGI_CF_ERROR)))))))))))))))));
        /* epilogue */
        _(~0U);
        return buf;

diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index a94d2732e32cbb586a939b9e6c044de5f0b9a7ae..597fbcbbdc6dd86b81f7af3fc9d8321a6a1b6f1b 100644 (file)
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -452,10 +452,10 @@ static void fcgi_trace(enum trace_level level, uint64_t mask, const struct trace
  */
 static inline int fcgi_recv_allowed(const struct fcgi_conn *fconn)
 {
-       if (b_data(&fconn->dbuf) == 0 &&
-           (fconn->state == FCGI_CS_CLOSED ||
-            fconn->conn->flags & CO_FL_ERROR ||
-            conn_xprt_read0_pending(fconn->conn)))
+       if (fconn->flags & (FCGI_CF_EOS|FCGI_CF_ERROR))
+               return 0;
+
+       if (b_data(&fconn->dbuf) == 0 && fconn->state == FCGI_CS_CLOSED)
                return 0;
 
        if (!(fconn->flags & FCGI_CF_DEM_DALLOC) &&
@@ -793,7 +793,7 @@ static void fcgi_release(struct fcgi_conn *fconn)
  */
 static int fcgi_conn_read0_pending(struct fcgi_conn *fconn)
 {
-       if (conn_xprt_read0_pending(fconn->conn) && !b_data(&fconn->dbuf))
+       if ((fconn->flags & FCGI_CF_EOS) && !b_data(&fconn->dbuf))
                return 1;
        return 0;
 }
@@ -804,12 +804,12 @@ static inline int fcgi_conn_is_dead(struct fcgi_conn *fconn)
 {
        if (eb_is_empty(&fconn->streams_by_id) &&               /* don't close if streams exist */
            (!(fconn->flags & FCGI_CF_KEEP_CONN) ||             /* don't keep the connection alive */
-            (fconn->conn->flags & CO_FL_ERROR) ||              /* errors close immediately */
+            (fconn->flags & FCGI_CF_ERROR) ||                  /* errors close immediately */
             (fconn->state == FCGI_CS_CLOSED && !fconn->task) ||/* a timeout stroke earlier */
             (!(fconn->conn->owner)) ||                         /* Nobody's left to take care of the connection, drop it now */
             (!br_data(fconn->mbuf) &&                          /* mux buffer empty, also process clean events below */
-             conn_xprt_read0_pending(fconn->conn))))
-               return 1;
+             (fconn->flags & FCGI_CF_EOS))))
+             return 1;
        return 0;
 }
 
@@ -1077,10 +1077,8 @@ static void fcgi_strm_wake_one_stream(struct fcgi_strm *fstrm)
                        fcgi_strm_close(fstrm);
        }
 
-       if ((fconn->state == FCGI_CS_CLOSED || fconn->conn->flags & CO_FL_ERROR)) {
-               se_fl_set(fstrm->sd, SE_FL_ERR_PENDING);
-               if (se_fl_test(fstrm->sd, SE_FL_EOS))
-                       se_fl_set(fstrm->sd, SE_FL_ERROR);
+       if (fconn->state == FCGI_CS_CLOSED || (fconn->flags & (FCGI_CF_ERR_PENDING|FCGI_CF_ERROR))) {
+               se_fl_set_error(fstrm->sd);
 
                if (fstrm->state < FCGI_SS_ERROR) {
                        fstrm->state = FCGI_SS_ERROR;
@@ -2755,10 +2753,18 @@ static int fcgi_recv(struct fcgi_conn *fconn)
        else
                TRACE_DATA("recv data", FCGI_EV_FCONN_RECV, conn, 0, 0, (size_t[]){ret});
 
+       if (conn_xprt_read0_pending(conn)) {
+               TRACE_DATA("received read0", FCGI_EV_FCONN_RECV, conn);
+               fconn->flags |= FCGI_CF_EOS;
+       }
+       if (conn->flags & CO_FL_ERROR) {
+               TRACE_DATA("connection error", FCGI_EV_FCONN_RECV, conn);
+               fconn->flags |= FCGI_CF_ERROR;
+       }
+
        if (!b_data(buf)) {
                fcgi_release_buf(fconn, &fconn->dbuf);
-               TRACE_LEAVE(FCGI_EV_FCONN_RECV, conn);
-               return (conn->flags & CO_FL_ERROR || conn_xprt_read0_pending(conn));
+               goto end;
        }
 
        if (ret == max) {
@@ -2766,8 +2772,9 @@ static int fcgi_recv(struct fcgi_conn *fconn)
                fconn->flags |= FCGI_CF_DEM_DFULL;
        }
 
+end:
        TRACE_LEAVE(FCGI_EV_FCONN_RECV, conn);
-       return !!ret || (conn->flags & CO_FL_ERROR) || conn_xprt_read0_pending(conn);
+       return !!ret || (fconn->flags & (FCGI_CF_EOS|FCGI_CF_ERROR));
 }
 
 
@@ -2782,8 +2789,11 @@ static int fcgi_send(struct fcgi_conn *fconn)
 
        TRACE_ENTER(FCGI_EV_FCONN_SEND, conn);
 
-       if (conn->flags & CO_FL_ERROR) {
+       if (fconn->flags & (FCGI_CF_ERROR|FCGI_CF_ERR_PENDING)) {
                TRACE_DEVEL("leaving on connection error", FCGI_EV_FCONN_SEND, conn);
+               if (fconn->flags & FCGI_CF_EOS)
+                       fconn->flags |= FCGI_CF_ERROR;
+               b_reset(br_tail(fconn->mbuf));
                return 1;
        }
 
@@ -2861,10 +2871,13 @@ static int fcgi_send(struct fcgi_conn *fconn)
                fconn->flags &= ~(FCGI_CF_MUX_MFULL | FCGI_CF_DEM_MROOM);
        }
 
-       if (conn->flags & CO_FL_SOCK_WR_SH) {
-               /* output closed, nothing to send, clear the buffer to release it */
+       if (conn->flags & CO_FL_ERROR) {
+               fconn->flags |= FCGI_CF_ERR_PENDING;
+               if (fconn->flags & FCGI_CF_EOS)
+                       fconn->flags |= FCGI_CF_ERROR;
                b_reset(br_tail(fconn->mbuf));
        }
+
        /* We're not full anymore, so we can wake any task that are waiting
         * for us.
         */
@@ -2905,7 +2918,7 @@ static int fcgi_send(struct fcgi_conn *fconn)
        /* We're done, no more to send */
        if (!br_data(fconn->mbuf)) {
                TRACE_DEVEL("leaving with everything sent", FCGI_EV_FCONN_SEND, conn);
-               return sent;
+               goto end;
        }
 schedule:
        if (!(conn->flags & CO_FL_ERROR) && !(fconn->wait_event.events & SUB_RETRY_SEND)) {
@@ -2914,7 +2927,8 @@ schedule:
        }
 
        TRACE_DEVEL("leaving with some data left to send", FCGI_EV_FCONN_SEND, conn);
-       return sent;
+end:
+       return sent || (fconn->flags & (FCGI_CF_ERR_PENDING|FCGI_CF_ERROR));
 }
 
 /* this is the tasklet referenced in fconn->wait_event.tasklet */
@@ -2996,7 +3010,7 @@ static int fcgi_process(struct fcgi_conn *fconn)
        if (b_data(&fconn->dbuf) && !(fconn->flags & FCGI_CF_DEM_BLOCK_ANY)) {
                fcgi_process_demux(fconn);
 
-               if (fconn->state == FCGI_CS_CLOSED || conn->flags & CO_FL_ERROR)
+               if (fconn->state == FCGI_CS_CLOSED || (fconn->flags & FCGI_CF_ERROR))
                        b_reset(&fconn->dbuf);
 
                if (buf_room_for_htx_data(&fconn->dbuf))
@@ -3036,7 +3050,7 @@ static int fcgi_process(struct fcgi_conn *fconn)
                }
        }
 
-       if ((conn->flags & CO_FL_ERROR) || fcgi_conn_read0_pending(fconn) ||
+       if ((fconn->flags & FCGI_CF_ERROR) || fcgi_conn_read0_pending(fconn) ||
            fconn->state == FCGI_CS_CLOSED || (fconn->flags & FCGI_CF_ABRTS_FAILED) ||
            eb_is_empty(&fconn->streams_by_id)) {
                fcgi_wake_some_streams(fconn, 0);
@@ -3052,8 +3066,7 @@ static int fcgi_process(struct fcgi_conn *fconn)
        if (!b_data(&fconn->dbuf))
                fcgi_release_buf(fconn, &fconn->dbuf);
 
-       if ((conn->flags & CO_FL_SOCK_WR_SH) ||
-           fconn->state == FCGI_CS_CLOSED  || (fconn->flags & FCGI_CF_ABRTS_FAILED) ||
+       if (fconn->state == FCGI_CS_CLOSED  || (fconn->flags & FCGI_CF_ABRTS_FAILED) ||
            (!br_data(fconn->mbuf) && ((fconn->flags & FCGI_CF_MUX_BLOCK_ANY) || LIST_ISEMPTY(&fconn->send_list))))
                fcgi_release_mbuf(fconn);
 
@@ -3526,7 +3539,7 @@ static void fcgi_detach(struct sedesc *sd)
        /* this stream may be blocked waiting for some data to leave, so orphan
         * it in this case.
         */
-       if (!(fconn->conn->flags & CO_FL_ERROR) &&
+       if (!(fconn->flags & (FCGI_CF_ERR_PENDING|FCGI_CF_ERROR)) && // FIXME: Be sure for ERR_PENDING
            (fconn->state != FCGI_CS_CLOSED) &&
            (fstrm->flags & (FCGI_SF_BLK_MBUSY|FCGI_SF_BLK_MROOM)) &&
            (fstrm->subs || (fstrm->flags & (FCGI_SF_WANT_SHUTR|FCGI_SF_WANT_SHUTW)))) {
@@ -3542,7 +3555,7 @@ static void fcgi_detach(struct sedesc *sd)
 
        fcgi_strm_destroy(fstrm);
 
-       if (!(fconn->conn->flags & (CO_FL_ERROR|CO_FL_SOCK_RD_SH|CO_FL_SOCK_WR_SH)) &&
+       if (!(fconn->flags & (FCGI_CF_EOS|FCGI_CF_ERR_PENDING|FCGI_CF_ERROR)) &&
            (fconn->flags & FCGI_CF_KEEP_CONN)) {
                if (fconn->conn->flags & CO_FL_PRIVATE) {
                        /* Add the connection in the session serverlist, if not already done */