]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: sessions: Don't be responsible for connections anymore.
authorOlivier Houchard <ohouchard@haproxy.com>
Mon, 20 Jan 2020 12:56:01 +0000 (13:56 +0100)
committerOlivier Houchard <cognet@ci0.org>
Thu, 19 Mar 2020 21:07:33 +0000 (22:07 +0100)
Make it so sessions are not responsible for connection anymore, except for
connections that are private, and thus can't be shared, otherwise, as soon
as a request is done, the session will just add the connection to the
orphan connections pool.
This will break http-reuse safe, but it is expected to be fixed later.

include/proto/session.h
src/backend.c
src/mux_fcgi.c
src/mux_h1.c
src/mux_h2.c
src/session.c

index 3eaa35753058c81f6402e4d36a3e35d6402f8ed3..b6f31f1765af9e30f78e68c5b7b1e310f6cf03df 100644 (file)
@@ -121,17 +121,13 @@ static inline int session_add_conn(struct session *sess, struct connection *conn
 /* Returns 0 if the session can keep the idle conn, -1 if it was destroyed, or 1 if it was added to the server list */
 static inline int session_check_idle_conn(struct session *sess, struct connection *conn)
 {
-       if (sess->idle_conns >= sess->fe->max_out_conns) {
-               /* We can't keep the connection, let's try to add it to the server idle list */
+       if (!(conn->flags & CO_FL_PRIVATE) ||
+           sess->idle_conns >= sess->fe->max_out_conns) {
                session_unown_conn(sess, conn);
                conn->owner = NULL;
                conn->flags &= ~CO_FL_SESS_IDLE;
-               if (!srv_add_to_idle_list(objt_server(conn->target), conn)) {
-                       /* The server doesn't want it, let's kill the connection right away */
-                       conn->mux->destroy(conn->ctx);
-                       return -1;
-               }
-               return 1;
+               conn->mux->destroy(conn);
+               return -1;
        } else {
                conn->flags |= CO_FL_SESS_IDLE;
                sess->idle_conns++;
index 66f9cf2fcd533374840d4ee2fa33aa2e17e40031..63f59617f8c46bc6d5cff2e65dc2e7ebac67eb5f 100644 (file)
@@ -1093,14 +1093,12 @@ int connect_server(struct stream *s)
 {
        struct connection *cli_conn = objt_conn(strm_orig(s));
        struct connection *srv_conn = NULL;
-       struct connection *old_conn = NULL;
        struct conn_stream *srv_cs = NULL;
        struct sess_srv_list *srv_list;
        struct server *srv;
        int reuse = 0;
        int reuse_orphan = 0;
        int init_mux = 0;
-       int alloced_cs = 0;
        int err;
 
 
@@ -1123,26 +1121,8 @@ int connect_server(struct stream *s)
                }
        }
 
-       if (!reuse) {
-               /* no connection was found in our session's list. Pick any
-                * random one that we could trade against another one.
-                */
+       if (!reuse)
                srv_conn = NULL;
-               if (!LIST_ISEMPTY(&s->sess->srv_list)) {
-                       srv_list = LIST_ELEM(s->sess->srv_list.n, struct sess_srv_list *, srv_list);
-                       if (!LIST_ISEMPTY(&srv_list->conn_list))
-                               srv_conn = LIST_ELEM(srv_list->conn_list.n, struct connection *, session_list);
-               }
-
-       }
-       /* OK at this point we have this :
-        *   - srv_conn points to an existing connection or NULL
-        *   - if reuse is set, srv_conn holds a valid connection, otherwise it
-        *     points to any of our old connections we may want to trade against
-        *     another one
-        */
-
-       old_conn = srv_conn;
 
        srv = objt_server(s->target);
 
@@ -1283,29 +1263,6 @@ int connect_server(struct stream *s)
                }
        }
 
-       /* We're about to use another connection, let the mux know we're
-        * done with this one.
-        */
-       if (old_conn != srv_conn && old_conn && reuse && !reuse_orphan) {
-               struct session *sess = srv_conn->owner;
-
-               if (sess) {
-                       if (old_conn && !(old_conn->flags & CO_FL_PRIVATE) &&
-                           old_conn->mux != NULL) {
-                               if (old_conn->flags & CO_FL_SESS_IDLE)
-                                       s->sess->idle_conns--;
-                               session_unown_conn(s->sess, old_conn);
-                               old_conn->owner = sess;
-                               if (!session_add_conn(sess, old_conn, old_conn->target)) {
-                                       old_conn->flags &= ~CO_FL_SESS_IDLE;
-                                       old_conn->owner = NULL;
-                                       old_conn->mux->destroy(old_conn->ctx);
-                               } else
-                                       session_check_idle_conn(sess, old_conn);
-                       }
-               }
-       }
-
        if (reuse) {
                if (srv_conn->mux) {
                        int avail = srv_conn->mux->avail_streams(srv_conn);
@@ -1318,10 +1275,9 @@ int connect_server(struct stream *s)
 
                        if (avail >= 1) {
                                srv_cs = srv_conn->mux->attach(srv_conn, s->sess);
-                               if (srv_cs) {
-                                       alloced_cs = 1;
+                               if (srv_cs)
                                        si_attach_cs(&s->si[1], srv_cs);
-                               else
+                               else
                                        srv_conn = NULL;
                        }
                        else
@@ -1340,24 +1296,6 @@ int connect_server(struct stream *s)
                srv_cs = NULL;
        }
 
-       if (srv_conn && old_conn != srv_conn) {
-               if (srv_conn->owner)
-                       session_unown_conn(srv_conn->owner, srv_conn);
-               srv_conn->owner = s->sess;
-               if (!session_add_conn(s->sess, srv_conn, srv_conn->target)) {
-                       /* If we failed to attach the connection, detach the
-                        * conn_stream, possibly destroying the connection */
-                       if (alloced_cs)
-                               si_release_endpoint(&s->si[1]);
-                       srv_conn->owner = NULL;
-                       if (srv_conn->mux && !srv_add_to_idle_list(objt_server(srv_conn->target), srv_conn))
-                       /* The server doesn't want it, let's kill the connection right away */
-                               srv_conn->mux->destroy(srv_conn->ctx);
-                       srv_conn = NULL;
-
-               }
-       }
-
        if (!srv_conn || !sockaddr_alloc(&srv_conn->dst))
                return SF_ERR_RESOURCE;
 
index 50cda636646fc48c86f9cf81ec338bbe8d155514..52bb7bac7f9124b9056e6ef87f4f60b84278da1c 100644 (file)
@@ -3476,46 +3476,37 @@ static void fcgi_detach(struct conn_stream *cs)
 
        if (!(fconn->conn->flags & (CO_FL_ERROR|CO_FL_SOCK_RD_SH|CO_FL_SOCK_WR_SH)) &&
            !(fconn->flags & FCGI_CF_KEEP_CONN)) {
-               if (!fconn->conn->owner) {
+               /* Never ever allow to reuse a connection from a non-reuse backend */
+               if ((fconn->proxy->options & PR_O_REUSE_MASK) == PR_O_REUSE_NEVR)
+                       fconn->conn->flags |= CO_FL_PRIVATE;
+               if (!fconn->conn->owner && (fconn->conn->flags & CO_FL_PRIVATE)) {
                        fconn->conn->owner = sess;
                        if (!session_add_conn(sess, fconn->conn, fconn->conn->target)) {
                                fconn->conn->owner = NULL;
                                if (eb_is_empty(&fconn->streams_by_id)) {
-                                       if (!srv_add_to_idle_list(objt_server(fconn->conn->target), fconn->conn)) {
-                                               /* The server doesn't want it, let's kill the connection right away */
-                                               fconn->conn->mux->destroy(fconn);
-                                               TRACE_DEVEL("outgoing connection killed", FCGI_EV_STRM_END|FCGI_EV_FCONN_ERR);
-                                       }
-                                       TRACE_DEVEL("reusable idle connection", FCGI_EV_STRM_END, fconn->conn);
-                                       return;
+                                       /* let's kill the connection right away */
+                                       fconn->conn->mux->destroy(fconn);
+                                       TRACE_DEVEL("outgoing connection killed", FCGI_EV_STRM_END|FCGI_EV_FCONN_ERR);
                                }
                        }
                }
                if (eb_is_empty(&fconn->streams_by_id)) {
-                       int ret = session_check_idle_conn(fconn->conn->owner, fconn->conn);
-                       if (ret == -1) {
+                       if (sess && fconn->conn->owner == sess &&
+                           session_check_idle_conn(fconn->conn->owner, fconn->conn) != 0) {
                                /* The connection is destroyed, let's leave */
                                TRACE_DEVEL("outgoing connection killed", FCGI_EV_STRM_END|FCGI_EV_FCONN_ERR);
                                return;
                        }
-                       else if (ret == 1) {
-                               /* The connection was added to the server idle list, just stop */
+                       if (!(fconn->conn->flags & CO_FL_PRIVATE)) {
+                               if (!srv_add_to_idle_list(objt_server(fconn->conn->target), fconn->conn)) {
+                                       /* The server doesn't want it, let's kill the connection right away */
+                                       fconn->conn->mux->destroy(fconn);
+                                       TRACE_DEVEL("outgoing connection killed", FCGI_EV_STRM_END|FCGI_EV_FCONN_ERR);
+                                       return;
+                               }
                                TRACE_DEVEL("reusable idle connection", FCGI_EV_STRM_END, fconn->conn);
                                return;
                        }
-                       TRACE_DEVEL("connection in idle session list", FCGI_EV_STRM_END, fconn->conn);
-               }
-               /* Never ever allow to reuse a connection from a non-reuse backend */
-               if ((fconn->proxy->options & PR_O_REUSE_MASK) == PR_O_REUSE_NEVR)
-                       fconn->conn->flags |= CO_FL_PRIVATE;
-               if (!LIST_ADDED(&fconn->conn->list) && fconn->nb_streams < fconn->streams_limit) {
-                       struct server *srv = objt_server(fconn->conn->target);
-
-                       if (srv) {
-                               if (!(fconn->conn->flags & CO_FL_PRIVATE))
-                                       LIST_ADD(&srv->idle_conns[tid], &fconn->conn->list);
-                       }
-                       TRACE_DEVEL("connection in idle server list", FCGI_EV_STRM_END, fconn->conn);
                }
        }
 
index b3c954e0e559dd5fd34e8b667b0e81463cb9292e..3504a23dca544b1a0568a6befe760c5522064b73 100644 (file)
@@ -2360,7 +2360,9 @@ static void h1_detach(struct conn_stream *cs)
        struct h1s *h1s = cs->ctx;
        struct h1c *h1c;
        struct session *sess;
+#if 0
        int is_not_first;
+#endif
 
        TRACE_ENTER(H1_EV_STRM_END, h1s ? h1s->h1c->conn : NULL, h1s);
 
@@ -2374,7 +2376,9 @@ static void h1_detach(struct conn_stream *cs)
        h1c = h1s->h1c;
        h1s->cs = NULL;
 
+#if 0
        is_not_first = h1s->flags & H1S_F_NOT_FIRST;
+#endif
        h1s_destroy(h1s);
 
        if (conn_is_back(h1c->conn) && (h1c->flags & H1C_F_CS_IDLE)) {
@@ -2395,51 +2399,30 @@ static void h1_detach(struct conn_stream *cs)
                if ((h1c->px->options & PR_O_REUSE_MASK) == PR_O_REUSE_NEVR)
                        h1c->conn->flags |= CO_FL_PRIVATE;
 
-               if (!(h1c->conn->owner)) {
+               if (!(h1c->conn->owner) && (h1c->conn->flags & CO_FL_PRIVATE)) {
                        h1c->conn->owner = sess;
                        if (!session_add_conn(sess, h1c->conn, h1c->conn->target)) {
                                h1c->conn->owner = NULL;
-                               if (!srv_add_to_idle_list(objt_server(h1c->conn->target), h1c->conn)) {
-                                       /* The server doesn't want it, let's kill the connection right away */
-                                       h1c->conn->mux->destroy(h1c);
-                                       TRACE_DEVEL("outgoing connection killed", H1_EV_STRM_END|H1_EV_H1C_END);
-                                       goto end;
-                               }
-                               h1c->conn->xprt->subscribe(h1c->conn, h1c->conn->xprt_ctx, SUB_RETRY_RECV, &h1c->wait_event);
-                               TRACE_DEVEL("reusable idle connection", H1_EV_STRM_END, h1c->conn);
+                               h1c->conn->mux->destroy(h1c);
                                goto end;
                        }
-               }
-               if (h1c->conn->owner == sess) {
-                       int ret = session_check_idle_conn(sess, h1c->conn);
-                       if (ret == -1) {
+                       if (session_check_idle_conn(sess, h1c->conn)) {
                                /* The connection got destroyed, let's leave */
                                TRACE_DEVEL("outgoing connection killed", H1_EV_STRM_END|H1_EV_H1C_END);
                                goto end;
                        }
-                       else if (ret == 1) {
-                               /* The connection was added to the server list,
-                                * wake the task so we can subscribe to events
-                                */
-                               h1c->conn->xprt->subscribe(h1c->conn, h1c->conn->xprt_ctx, SUB_RETRY_RECV, &h1c->wait_event);
-                               TRACE_DEVEL("reusable idle connection", H1_EV_STRM_END, h1c->conn);
-                               goto end;
-                       }
-                       TRACE_DEVEL("connection in idle session list", H1_EV_STRM_END, h1c->conn);
                }
-               /* we're in keep-alive with an idle connection, monitor it if not already done */
-               if (LIST_ISEMPTY(&h1c->conn->list)) {
-                       struct server *srv = objt_server(h1c->conn->target);
-
-                       if (srv) {
-                               if (!(h1c->conn->flags & CO_FL_PRIVATE)) {
-                                       if (is_not_first)
-                                               LIST_ADD(&srv->safe_conns[tid], &h1c->conn->list);
-                                       else
-                                               LIST_ADD(&srv->idle_conns[tid], &h1c->conn->list);
-                               }
-                               TRACE_DEVEL("connection in idle server list", H1_EV_STRM_END, h1c->conn);
+               if (!(h1c->conn->flags & CO_FL_PRIVATE)) {
+                       if (h1c->conn->owner == sess)
+                               h1c->conn->owner = NULL;
+                       if (!srv_add_to_idle_list(objt_server(h1c->conn->target), h1c->conn)) {
+                               /* The server doesn't want it, let's kill the connection right away */
+                               h1c->conn->mux->destroy(h1c);
+                               TRACE_DEVEL("outgoing connection killed", H1_EV_STRM_END|H1_EV_H1C_END);
+                               goto end;
                        }
+                       h1c->conn->xprt->subscribe(h1c->conn, h1c->conn->xprt_ctx, SUB_RETRY_RECV, &h1c->wait_event);
+                       return;
                }
        }
 
index 013ef86f819af3fde3467653f1e302603d2488eb..58af4b1b6ec4af7c1f10475f5d6b453c6f3ff1c5 100644 (file)
@@ -3874,37 +3874,38 @@ static void h2_detach(struct conn_stream *cs)
        if (h2c->flags & H2_CF_IS_BACK) {
                if (!(h2c->conn->flags &
                    (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_SOCK_WR_SH))) {
-                       if (!h2c->conn->owner) {
+                       /* Never ever allow to reuse a connection from a non-reuse backend */
+                       if ((h2c->proxy->options & PR_O_REUSE_MASK) == PR_O_REUSE_NEVR)
+                               h2c->conn->flags |= CO_FL_PRIVATE;
+                       if (!h2c->conn->owner && (h2c->conn->flags & CO_FL_PRIVATE)) {
                                h2c->conn->owner = sess;
                                if (!session_add_conn(sess, h2c->conn, h2c->conn->target)) {
                                        h2c->conn->owner = NULL;
                                        if (eb_is_empty(&h2c->streams_by_id)) {
-                                               if (!srv_add_to_idle_list(objt_server(h2c->conn->target), h2c->conn))
-                                                       /* The server doesn't want it, let's kill the connection right away */
-                                                       h2c->conn->mux->destroy(h2c);
+                                               h2c->conn->mux->destroy(h2c);
                                                TRACE_DEVEL("leaving on error after killing outgoing connection", H2_EV_STRM_END|H2_EV_H2C_ERR);
                                                return;
                                        }
                                }
                        }
                        if (eb_is_empty(&h2c->streams_by_id)) {
-                               if (session_check_idle_conn(h2c->conn->owner, h2c->conn) != 0) {
+                               if (sess && h2c->conn->owner == sess &&
+                                   session_check_idle_conn(h2c->conn->owner, h2c->conn) != 0) {
                                        /* At this point either the connection is destroyed, or it's been added to the server idle list, just stop */
                                        TRACE_DEVEL("leaving without reusable idle connection", H2_EV_STRM_END);
                                        return;
                                }
-                       }
-                       /* Never ever allow to reuse a connection from a non-reuse backend */
-                       if ((h2c->proxy->options & PR_O_REUSE_MASK) == PR_O_REUSE_NEVR)
-                               h2c->conn->flags |= CO_FL_PRIVATE;
-                       if (!LIST_ADDED(&h2c->conn->list) && h2c->nb_streams < h2c->streams_limit) {
-                               struct server *srv = objt_server(h2c->conn->target);
+                               if (!(h2c->conn->flags & CO_FL_PRIVATE)) {
+                                       if (!srv_add_to_idle_list(objt_server(h2c->conn->target), h2c->conn)) {
+                                               /* The server doesn't want it, let's kill the connection right away */
+                                               h2c->conn->mux->destroy(h2c);
+                                               TRACE_DEVEL("leaving on error after killing outgoing connection", H2_EV_STRM_END|H2_EV_H2C_ERR);
+                                               return;
+                                       }
+                                       TRACE_DEVEL("reusable idle connection", H2_EV_STRM_END);
+                                       return;
 
-                               if (srv) {
-                                       if (!(h2c->conn->flags & CO_FL_PRIVATE))
-                                               LIST_ADD(&srv->idle_conns[tid], &h2c->conn->list);
                                }
-
                        }
                }
        }
index c62cc1097e0196dc853bee8b8dc37847e616eddb..0c8e344fcffec61c2cd39ee1ab365fec8ec696de 100644 (file)
@@ -82,8 +82,7 @@ void session_free(struct session *sess)
                        if (conn->mux) {
                                conn->owner = NULL;
                                conn->flags &= ~CO_FL_SESS_IDLE;
-                               if (!srv_add_to_idle_list(objt_server(conn->target), conn))
-                                       conn->mux->destroy(conn->ctx);
+                               conn->mux->destroy(conn->ctx);
                        } else {
                                /* We have a connection, but not yet an associated mux.
                                 * So destroy it now.