BUG/MEDIUM: conn-stream: Set back CS to RDY state when the appctx is created

When an appctx is created on the server side, we now set the corresponding
conn-stream to ready state (CS_ST_RDY). When it happens, the backend
conn-stream is in CS_ST_INI state. It is not consistant to let the
conn-stream in this state because it means it is possible to have a target
installed in CS_ST_INI state, while with a connection, the conn-stream is
switch to CS_ST_RDY or CS_ST_EST state.

It is especially anbiguous because we may be tempted to think there is no
endpoint attached to the conn-stream before the CS_ST_CON state. And it is
indeed the reason for a bug leading to a crash because a cs_detach_endp() is
performed if an abort is detected on the backend conn-stream in CS_ST_INI
state. With a mux or a appctx attached to the conn-stream, "->endp" field is
set to NULL. It is unexpected. The API will be changed to be sure it is not
possible. But it exposes a consistency issue with applets.

So, the conn-stream must not stay in CS_ST_INI state when an appctx is
attached. But there is no reason to set it in CS_ST_REQ. The conn-stream
must be set to CS_ST_RDY to handle applets and connections in the same
way. Note that if only the target is set but no appctx is created, the
backend conn-stream is switched from CS_ST_INI to CS_ST_REQ state to be able
to create the corresponding appctx. This part is unchanged.

This patch depends on the commit "MINOR: backend: Don't allow to change
backend applet".

The ambiguity exists on previous versions. But the issue is
2.6-specific. Thus, no backport is needed.

diff --git a/src/backend.c b/src/backend.c
index 12f9a651b64be9db46335a2343e15f40ad7c0b53..6bff6ffe5d3908d01bfef452f6aeff5aa280a29f 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -2113,7 +2113,7 @@ abort_connection:
 /* This function initiates a server connection request on a conn-stream
  * already in CS_ST_REQ state. Upon success, the state goes to CS_ST_ASS for
  * a real connection to a server, indicating that a server has been assigned,
- * or CS_ST_EST for a successful connection to an applet. It may also return
+ * or CS_ST_RDY for a successful connection to an applet. It may also return
  * CS_ST_QUE, or CS_ST_CLO upon error.
  */
 void back_handle_st_req(struct stream *s)
@@ -2126,12 +2126,14 @@ void back_handle_st_req(struct stream *s)
        DBG_TRACE_ENTER(STRM_EV_STRM_PROC|STRM_EV_CS_ST, s);
 
        if (unlikely(obj_type(s->target) == OBJ_TYPE_APPLET)) {
-               /* the applet directly goes to the EST state */
-               struct appctx *appctx = cs_appctx(s->csb);
-
-               if (!appctx)
-                       appctx = cs_applet_create(cs, objt_applet(s->target));
+               struct appctx *appctx;
 
+               /* The target is an applet but the CS is in CS_ST_REQ. Thus it
+                * means no appctx are attached to the CS. Otherwise, it will be
+                * in CS_ST_RDY state. So, try to create the appctx now.
+                */
+               BUG_ON(cs_appctx(cs));
+               appctx = cs_applet_create(cs, objt_applet(s->target));
                if (!appctx) {
                        /* No more memory, let's immediately abort. Force the
                         * error code to ignore the ERR_LOCAL which is not a
@@ -2150,15 +2152,7 @@ void back_handle_st_req(struct stream *s)
                        goto end;
                }
 
-               if (tv_iszero(&s->logs.tv_request))
-                       s->logs.tv_request = now;
-               s->logs.t_queue   = tv_ms_elapsed(&s->logs.tv_accept, &now);
-               cs->state     = CS_ST_EST;
-               s->conn_err_type  = STRM_ET_NONE;
-               be_set_sess_last(s->be);
-
                DBG_TRACE_STATE("applet registered", STRM_EV_STRM_PROC|STRM_EV_CS_ST, s);
-               /* let back_establish() finish the job */
                goto end;
        }
 
@@ -2406,6 +2400,19 @@ void back_handle_st_rdy(struct stream *s)
        struct channel *rep = &s->res;
 
        DBG_TRACE_ENTER(STRM_EV_STRM_PROC|STRM_EV_CS_ST, s);
+
+       if (unlikely(obj_type(s->target) == OBJ_TYPE_APPLET)) {
+               /* Here the appctx must exists because the CS was set to
+                * CS_ST_RDY state when the appctx was created.
+                */
+               BUG_ON(!cs_appctx(s->csb));
+
+               if (tv_iszero(&s->logs.tv_request))
+                       s->logs.tv_request = now;
+               s->logs.t_queue   = tv_ms_elapsed(&s->logs.tv_accept, &now);
+               be_set_sess_last(s->be);
+       }
+
        /* We know the connection at least succeeded, though it could have
         * since met an error for any other reason. At least it didn't time out
         * even though the timeout might have been reported right after success.

diff --git a/src/conn_stream.c b/src/conn_stream.c
index 29bf49b050511463d3f2be0847615936709b6b14..e753749ca92d535810cec55cf9d16b7b3a637551 100644 (file)
--- a/src/conn_stream.c
+++ b/src/conn_stream.c
@@ -468,6 +468,8 @@ struct appctx *cs_applet_create(struct conn_stream *cs, struct applet *app)
        appctx->t->nice = __cs_strm(cs)->task->nice;
        cs_cant_get(cs);
        appctx_wakeup(appctx);
+
+       cs->state = CS_ST_RDY;
        return appctx;
 }