diff --git a/include/haproxy/queue.h b/include/haproxy/queue.h index f61181a3a..a5d52b3fb 100644 --- a/include/haproxy/queue.h +++ b/include/haproxy/queue.h @@ -34,7 +34,7 @@ extern struct pool_head *pool_head_pendconn; struct pendconn *pendconn_add(struct stream *strm); int pendconn_dequeue(struct stream *strm); -void process_srv_queue(struct server *s); +void process_srv_queue(struct server *s, int server_locked); unsigned int srv_dynamic_maxconn(const struct server *s); int pendconn_redistribute(struct server *s); int pendconn_grab_from_px(struct server *s); diff --git a/include/haproxy/stream.h b/include/haproxy/stream.h index 9a7ffa1d2..d313d2c42 100644 --- a/include/haproxy/stream.h +++ b/include/haproxy/stream.h @@ -339,7 +339,7 @@ static inline void stream_choose_redispatch(struct stream *s) ((s->be->lbprm.algo & BE_LB_KIND) == BE_LB_KIND_RR)))) { sess_change_server(s, NULL); if (may_dequeue_tasks(objt_server(s->target), s->be)) - process_srv_queue(objt_server(s->target)); + process_srv_queue(objt_server(s->target), 0); s->flags &= ~(SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET); si->state = SI_ST_REQ; diff --git a/src/backend.c b/src/backend.c index 2ce34d1e1..9307bac8f 100644 --- a/src/backend.c +++ b/src/backend.c @@ -819,7 +819,7 @@ out_ok: sess_change_server(s, srv); } else { if (may_dequeue_tasks(conn_slot, s->be)) - process_srv_queue(conn_slot); + process_srv_queue(conn_slot, 0); } } @@ -1831,7 +1831,7 @@ int srv_redispatch_connect(struct stream *s) /* release other streams waiting for this server */ if (may_dequeue_tasks(srv, s->be)) - process_srv_queue(srv); + process_srv_queue(srv, 0); return 1; } /* if we get here, it's because we got SRV_STATUS_OK, which also @@ -1907,7 +1907,7 @@ void back_try_conn_req(struct stream *s) /* release other streams waiting for this server */ sess_change_server(s, NULL); if (may_dequeue_tasks(srv, s->be)) - process_srv_queue(srv); + process_srv_queue(srv, 0); /* Failed and not retryable. */ si_shutr(si); @@ -2235,7 +2235,7 @@ void back_handle_st_cer(struct stream *s) _HA_ATOMIC_INC(&s->be->be_counters.failed_conns); sess_change_server(s, NULL); if (may_dequeue_tasks(objt_server(s->target), s->be)) - process_srv_queue(objt_server(s->target)); + process_srv_queue(objt_server(s->target), 0); /* shutw is enough so stop a connecting socket */ si_shutw(si); diff --git a/src/cli.c b/src/cli.c index bc561cf2c..00990dc47 100644 --- a/src/cli.c +++ b/src/cli.c @@ -2565,7 +2565,7 @@ int pcli_wait_for_response(struct stream *s, struct channel *rep, int an_bit) HA_ATOMIC_DEC(&__objt_server(s->target)->cur_sess); } if (may_dequeue_tasks(__objt_server(s->target), be)) - process_srv_queue(__objt_server(s->target)); + process_srv_queue(__objt_server(s->target), 0); } s->target = NULL; diff --git a/src/queue.c b/src/queue.c index 6c51dcd29..2c3b7215f 100644 --- a/src/queue.c +++ b/src/queue.c @@ -337,14 +337,16 @@ static int pendconn_process_next_strm(struct server *srv, struct proxy *px) } /* Manages a server's connection queue. This function will try to dequeue as - * many pending streams as possible, and wake them up. + * many pending streams as possible, and wake them up. must + * only be set if the caller already hold the server lock. */ -void process_srv_queue(struct server *s) +void process_srv_queue(struct server *s, int server_locked) { struct proxy *p = s->proxy; int maxconn; - HA_SPIN_LOCK(SERVER_LOCK, &s->lock); + if (!server_locked) + HA_SPIN_LOCK(SERVER_LOCK, &s->lock); HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock); maxconn = srv_dynamic_maxconn(s); while (s->served < maxconn) { @@ -353,7 +355,8 @@ void process_srv_queue(struct server *s) break; } HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &p->lock); - HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); + if (!server_locked) + HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); } /* Adds the stream to the pending connection queue of server ->srv diff --git a/src/server.c b/src/server.c index 96390e86d..141f1ba09 100644 --- a/src/server.c +++ b/src/server.c @@ -1812,16 +1812,17 @@ const char *server_parse_maxconn_change_request(struct server *sv, else if (end[0] != '\0') return "Trailing garbage in maxconn string"; - HA_SPIN_LOCK(SERVER_LOCK, &sv->lock); if (sv->maxconn == sv->minconn) { // static maxconn sv->maxconn = sv->minconn = v; } else { // dynamic maxconn sv->maxconn = v; } - HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock); + /* server_parse_maxconn_change_request requires the server lock held. + * Specify it to process_srv_queue to prevent a deadlock. + */ if (may_dequeue_tasks(sv, sv->proxy)) - process_srv_queue(sv); + process_srv_queue(sv, 1); return NULL; } @@ -4193,10 +4194,14 @@ static int cli_parse_set_maxconn_server(char **args, char *payload, struct appct if (!sv) return 1; + HA_SPIN_LOCK(SERVER_LOCK, &sv->lock); + warning = server_parse_maxconn_change_request(sv, args[4]); if (warning) cli_err(appctx, warning); + HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock); + return 1; } diff --git a/src/stream.c b/src/stream.c index 952c70f58..bb5a93e8d 100644 --- a/src/stream.c +++ b/src/stream.c @@ -623,7 +623,7 @@ static void stream_free(struct stream *s) _HA_ATOMIC_DEC(&__objt_server(s->target)->cur_sess); } if (may_dequeue_tasks(objt_server(s->target), s->be)) - process_srv_queue(objt_server(s->target)); + process_srv_queue(objt_server(s->target), 0); } if (unlikely(s->srv_conn)) { @@ -1815,7 +1815,7 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) } sess_change_server(s, NULL); if (may_dequeue_tasks(srv, s->be)) - process_srv_queue(srv); + process_srv_queue(srv, 0); } } diff --git a/src/tcpcheck.c b/src/tcpcheck.c index dd0a650e1..4e594a9f7 100644 --- a/src/tcpcheck.c +++ b/src/tcpcheck.c @@ -937,6 +937,9 @@ enum tcpcheck_eval_ret tcpcheck_agent_expect_reply(struct check *check, struct t cs += strlen("maxconn:"); TRACE_DEVEL("change server maxconn", CHK_EV_TCPCHK_EXP, check); + /* This is safe to call server_parse_maxconn_change_request + * because the server lock is held during the check. + */ msg = server_parse_maxconn_change_request(check->server, cs); if (!wrn || !*wrn) wrn = msg;