From: Olivier Houchard Date: Mon, 20 Jan 2020 12:56:01 +0000 (+0100) Subject: MEDIUM: sessions: Don't be responsible for connections anymore. X-Git-Tag: v2.2-dev5~28 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2444aa5b6;p=thirdparty%2Fhaproxy.git MEDIUM: sessions: Don't be responsible for connections anymore. 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. --- diff --git a/include/proto/session.h b/include/proto/session.h index 3eaa357530..b6f31f1765 100644 --- a/include/proto/session.h +++ b/include/proto/session.h @@ -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++; diff --git a/src/backend.c b/src/backend.c index 66f9cf2fcd..63f59617f8 100644 --- a/src/backend.c +++ b/src/backend.c @@ -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; diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c index 50cda63664..52bb7bac7f 100644 --- a/src/mux_fcgi.c +++ b/src/mux_fcgi.c @@ -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); } } diff --git a/src/mux_h1.c b/src/mux_h1.c index b3c954e0e5..3504a23dca 100644 --- a/src/mux_h1.c +++ b/src/mux_h1.c @@ -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; } } diff --git a/src/mux_h2.c b/src/mux_h2.c index 013ef86f81..58af4b1b6e 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -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); } - } } } diff --git a/src/session.c b/src/session.c index c62cc1097e..0c8e344fcf 100644 --- a/src/session.c +++ b/src/session.c @@ -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.