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 a321f62..4599a36 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 @@
_(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 a94d273..597fbcb 100644
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -452,12 +452,12 @@
*/
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) &&
!(fconn->flags & FCGI_CF_DEM_BLOCK_ANY))
return 1;
@@ -793,7 +793,7 @@
*/
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 @@
{
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
/* 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 @@
}
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 @@
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 @@
}
}
- 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 @@
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 @@
/* 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 @@
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 */