Revert "MEDIUM: queue: simplify again the process_srv_queue() API"
This reverts commit c83e45e9b001591633188a480a896c935d3c9625.
The recent changes since 5304669e1 MEDIUM: queue: make
pendconn_process_next_strm() only return the pendconn opened a tiny race
condition between stream_free() and process_srv_queue(), as the pendconn
is accessed outside of the lock, possibly while it's being freed. A
different approach is required.
diff --git a/include/haproxy/queue.h b/include/haproxy/queue.h
index 101a4ca..58a0b09 100644
--- a/include/haproxy/queue.h
+++ b/include/haproxy/queue.h
@@ -34,7 +34,7 @@
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 9a7ffa1..d313d2c 100644
--- a/include/haproxy/stream.h
+++ b/include/haproxy/stream.h
@@ -339,7 +339,7 @@
((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 f5fec1d..483ec54 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -818,7 +818,7 @@
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);
}
}
@@ -1830,7 +1830,7 @@
/* 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
@@ -1906,7 +1906,7 @@
/* 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);
@@ -2234,7 +2234,7 @@
_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 bc561cf..00990dc 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -2565,7 +2565,7 @@
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 618825e..78a4b52 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -331,9 +331,10 @@
}
/* 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. <server_locked> 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 done = 0;
diff --git a/src/server.c b/src/server.c
index 2b3cc82..0a128a2 100644
--- a/src/server.c
+++ b/src/server.c
@@ -1818,8 +1818,11 @@
sv->maxconn = v;
}
+ /* 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;
}
diff --git a/src/stream.c b/src/stream.c
index c9c01fc..b8da935 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -623,7 +623,7 @@
_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 @@
}
sess_change_server(s, NULL);
if (may_dequeue_tasks(srv, s->be))
- process_srv_queue(srv);
+ process_srv_queue(srv, 0);
}
}