From: Christopher Faulet Date: Wed, 12 Oct 2022 15:51:51 +0000 (+0200) Subject: MEDIUM: mux-fcgi: Introduce flags to deal with connection read/write errors X-Git-Tag: v2.7-dev9~22 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ab79b321d66b941f588351bcde8316324f9cb71b;p=thirdparty%2Fhaproxy.git 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 a321f62f53..4599a36da5 100644 --- 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 a94d2732e3..597fbcbbdc 100644 --- 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 */