MEDIUM: mux-fcgi: do not make an fstrm subscribe to itself on deferred shut
This is the port to FCGI of previous commit "MEDIUM: mux-h2: do not make
an h2s subscribe to itself on deferred shut".
The purpose is to avoid subscribing to the send_wait list when trying to
close, because we'll soon have to merge both recv and send lists. Basic
testing showed no difference (performance nor issues).
diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index 1b0495d..bad2e30 100644
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -164,10 +164,10 @@
struct buffer rxbuf; /* receive buffer, always valid (buf_empty or real buffer) */
struct eb32_node by_id; /* place in fcgi_conn's streams_by_id */
- struct wait_event wait_event; /* Wait list, when we're attempting to send an ABORT but we can't send */
struct wait_event *recv_wait; /* Address of the wait_event the conn_stream associated is waiting on */
struct wait_event *send_wait; /* Address of the wait_event the conn_stream associated is waiting on */
struct list send_list; /* To be used when adding in fcgi_conn->send_list */
+ struct tasklet *shut_tl; /* deferred shutdown tasklet, to retry to close after we failed to by lack of space */
};
/* Flags representing all default FCGI parameters */
@@ -935,6 +935,10 @@
tasklet_wakeup(sw->tasklet);
fstrm->send_wait = NULL;
}
+ else if (fstrm->flags & (FCGI_SF_WANT_SHUTR | FCGI_SF_WANT_SHUTW)) {
+ TRACE_POINT(FCGI_EV_STRM_WAKE, fstrm->fconn->conn, fstrm);
+ tasklet_wakeup(fstrm->shut_tl);
+ }
}
/* Alerts the data layer, trying to wake it up by all means, following
@@ -949,7 +953,8 @@
static void fcgi_strm_alert(struct fcgi_strm *fstrm)
{
TRACE_POINT(FCGI_EV_STRM_WAKE, fstrm->fconn->conn, fstrm);
- if (fstrm->recv_wait || fstrm->send_wait) {
+ if (fstrm->recv_wait || fstrm->send_wait ||
+ (fstrm->flags & (FCGI_SF_WANT_SHUTR|FCGI_SF_WANT_SHUTW))) {
fcgi_strm_notify_recv(fstrm);
fcgi_strm_notify_send(fstrm);
}
@@ -1022,7 +1027,7 @@
* we're in it, we're getting out anyway
*/
LIST_DEL_INIT(&fstrm->send_list);
- tasklet_free(fstrm->wait_event.tasklet);
+ tasklet_free(fstrm->shut_tl);
pool_free(pool_head_fcgi_strm, fstrm);
TRACE_LEAVE(FCGI_EV_FSTRM_END, conn);
@@ -1044,16 +1049,15 @@
if (!fstrm)
goto out;
- fstrm->wait_event.tasklet = tasklet_new();
- if (!fstrm->wait_event.tasklet) {
+ fstrm->shut_tl = tasklet_new();
+ if (!fstrm->shut_tl) {
pool_free(pool_head_fcgi_strm, fstrm);
goto out;
}
fstrm->send_wait = NULL;
fstrm->recv_wait = NULL;
- fstrm->wait_event.tasklet->process = fcgi_deferred_shut;
- fstrm->wait_event.tasklet->context = fstrm;
- fstrm->wait_event.events = 0;
+ fstrm->shut_tl->process = fcgi_deferred_shut;
+ fstrm->shut_tl->context = fstrm;
LIST_INIT(&fstrm->send_list);
fstrm->fconn = fconn;
fstrm->cs = NULL;
@@ -2646,18 +2650,25 @@
if (fstrm->flags & FCGI_SF_NOTIFIED)
continue;
- /* For some reason, the upper layer failed to subsribe again,
- * so remove it from the send_list
+ /* If the sender changed his mind and unsubscribed, let's just
+ * remove the stream from the send_list.
*/
- if (!fstrm->send_wait) {
+ if (!fstrm->send_wait &&
+ !(fstrm->flags & (FCGI_SF_WANT_SHUTR|FCGI_SF_WANT_SHUTW))) {
LIST_DEL_INIT(&fstrm->send_list);
continue;
}
- TRACE_POINT(FCGI_EV_STRM_WAKE, fconn->conn, fstrm);
- fstrm->flags &= ~FCGI_SF_BLK_ANY;
- fstrm->send_wait->events &= ~SUB_RETRY_SEND;
- fstrm->flags |= FCGI_SF_NOTIFIED;
- tasklet_wakeup(fstrm->send_wait->tasklet);
+ if (fstrm->send_wait) {
+ TRACE_POINT(FCGI_EV_STRM_WAKE, fconn->conn, fstrm);
+ fstrm->flags &= ~FCGI_SF_BLK_ANY;
+ fstrm->send_wait->events &= ~SUB_RETRY_SEND;
+ fstrm->flags |= FCGI_SF_NOTIFIED;
+ tasklet_wakeup(fstrm->send_wait->tasklet);
+ } else {
+ /* it's the shut request that was queued */
+ TRACE_POINT(FCGI_EV_STRM_WAKE, fconn->conn, fstrm);
+ tasklet_wakeup(fstrm->shut_tl);
+ }
}
fail:
@@ -2854,18 +2865,25 @@
if (fstrm->flags & FCGI_SF_NOTIFIED)
continue;
- /* For some reason, the upper layer failed to subsribe again,
- * so remove it from the send_list
+ /* If the sender changed his mind and unsubscribed, let's just
+ * remove the stream from the send_list.
*/
- if (!fstrm->send_wait) {
+ if (!fstrm->send_wait &&
+ !(fstrm->flags & (FCGI_SF_WANT_SHUTR|FCGI_SF_WANT_SHUTW))) {
LIST_DEL_INIT(&fstrm->send_list);
continue;
}
- fstrm->flags &= ~FCGI_SF_BLK_ANY;
- fstrm->send_wait->events &= ~SUB_RETRY_SEND;
- TRACE_DEVEL("waking up pending stream", FCGI_EV_FCONN_SEND|FCGI_EV_STRM_WAKE, conn, fstrm);
- tasklet_wakeup(fstrm->send_wait->tasklet);
- fstrm->flags |= FCGI_SF_NOTIFIED;
+ if (fstrm->send_wait) {
+ fstrm->flags &= ~FCGI_SF_BLK_ANY;
+ fstrm->send_wait->events &= ~SUB_RETRY_SEND;
+ TRACE_DEVEL("waking up pending stream", FCGI_EV_FCONN_SEND|FCGI_EV_STRM_WAKE, conn, fstrm);
+ tasklet_wakeup(fstrm->send_wait->tasklet);
+ fstrm->flags |= FCGI_SF_NOTIFIED;
+ } else {
+ /* it's the shut request that was queued */
+ TRACE_POINT(FCGI_EV_STRM_WAKE, fconn->conn, fstrm);
+ tasklet_wakeup(fstrm->shut_tl);
+ }
}
}
/* We're done, no more to send */
@@ -3429,7 +3447,8 @@
*/
if (!(cs->conn->flags & CO_FL_ERROR) &&
(fconn->state != FCGI_CS_CLOSED) &&
- (fstrm->flags & (FCGI_SF_BLK_MBUSY|FCGI_SF_BLK_MROOM)) && (fstrm->send_wait || fstrm->recv_wait)) {
+ (fstrm->flags & (FCGI_SF_BLK_MBUSY|FCGI_SF_BLK_MROOM)) &&
+ (fstrm->send_wait || fstrm->recv_wait || (fstrm->flags & (FCGI_SF_WANT_SHUTR|FCGI_SF_WANT_SHUTW)))) {
TRACE_DEVEL("leaving on stream blocked", FCGI_EV_STRM_END|FCGI_EV_FSTRM_BLK, fconn->conn, fstrm);
return;
}
@@ -3511,7 +3530,6 @@
static void fcgi_do_shutr(struct fcgi_strm *fstrm)
{
struct fcgi_conn *fconn = fstrm->fconn;
- struct wait_event *sw = &fstrm->wait_event;
TRACE_ENTER(FCGI_EV_STRM_SHUT, fconn->conn, fstrm);
@@ -3544,14 +3562,16 @@
return;
add_to_list:
+ /* Let the handler know we want to shutr, and add ourselves to the
+ * send list if not yet done. fcgi_deferred_shut() will be
+ * automatically called via the shut_tl tasklet when there's room
+ * again.
+ */
if (!LIST_ADDED(&fstrm->send_list)) {
- sw->events |= SUB_RETRY_SEND;
if (fstrm->flags & (FCGI_SF_BLK_MBUSY|FCGI_SF_BLK_MROOM)) {
- fstrm->send_wait = sw;
LIST_ADDQ(&fconn->send_list, &fstrm->send_list);
}
}
- /* Let the handler know we want shutr */
fstrm->flags |= FCGI_SF_WANT_SHUTR;
TRACE_LEAVE(FCGI_EV_STRM_SHUT, fconn->conn, fstrm);
return;
@@ -3561,7 +3581,6 @@
static void fcgi_do_shutw(struct fcgi_strm *fstrm)
{
struct fcgi_conn *fconn = fstrm->fconn;
- struct wait_event *sw = &fstrm->wait_event;
TRACE_ENTER(FCGI_EV_STRM_SHUT, fconn->conn, fstrm);
@@ -3599,20 +3618,22 @@
return;
add_to_list:
+ /* Let the handler know we want to shutr, and add ourselves to the
+ * send list if not yet done. fcgi_deferred_shut() will be
+ * automatically called via the shut_tl tasklet when there's room
+ * again.
+ */
if (!LIST_ADDED(&fstrm->send_list)) {
- sw->events |= SUB_RETRY_SEND;
if (fstrm->flags & (FCGI_SF_BLK_MBUSY|FCGI_SF_BLK_MROOM)) {
- fstrm->send_wait = sw;
LIST_ADDQ(&fconn->send_list, &fstrm->send_list);
}
}
- /* let the handler know we want to shutw */
fstrm->flags |= FCGI_SF_WANT_SHUTW;
TRACE_LEAVE(FCGI_EV_STRM_SHUT, fconn->conn, fstrm);
return;
}
-/* This is the tasklet referenced in fstrm->wait_event.tasklet, it is used for
+/* This is the tasklet referenced in fstrm->shut_tl, it is used for
* deferred shutdowns when the fcgi_detach() was done but the mux buffer was full
* and prevented the last record from being emitted.
*/
@@ -3623,7 +3644,11 @@
TRACE_ENTER(FCGI_EV_STRM_SHUT, fconn->conn, fstrm);
- fstrm->flags &= ~FCGI_SF_NOTIFIED;
+ if (fstrm->flags & FCGI_SF_NOTIFIED) {
+ /* some data processing remains to be done first */
+ goto end;
+ }
+
if (fstrm->flags & FCGI_SF_WANT_SHUTW)
fcgi_do_shutw(fstrm);
@@ -3640,7 +3665,7 @@
fcgi_release(fconn);
}
}
-
+ end:
TRACE_LEAVE(FCGI_EV_STRM_SHUT);
return NULL;
}
@@ -3730,8 +3755,8 @@
TRACE_DEVEL("subscribe(send)", FCGI_EV_STRM_SEND, fconn->conn, fstrm);
sw = param;
BUG_ON(fstrm->send_wait != sw);
- LIST_DEL(&fstrm->send_list);
- LIST_INIT(&fstrm->send_list);
+ if (!(fstrm->flags & (FCGI_SF_WANT_SHUTR|FCGI_SF_WANT_SHUTW)))
+ LIST_DEL_INIT(&fstrm->send_list);
sw->events &= ~SUB_RETRY_SEND;
fstrm->flags &= ~FCGI_SF_NOTIFIED;
fstrm->send_wait = NULL;
@@ -3929,7 +3954,8 @@
}
/* Ok we managed to send something, leave the send_list */
- LIST_DEL_INIT(&fstrm->send_list);
+ if (!(fstrm->flags & (FCGI_SF_WANT_SHUTR|FCGI_SF_WANT_SHUTW)))
+ LIST_DEL_INIT(&fstrm->send_list);
}
TRACE_LEAVE(FCGI_EV_STRM_SEND, fconn->conn, fstrm, htx, (size_t[]){total});