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 101a4ca5f61b696fbd830b15a51077a1d13ae8e8..58a0b091ca5cc82ae7656378c6b5df6f2332388d 100644 (file)
--- 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 9a7ffa1d2df8fb4a8209d64e3a8df25d2bc2ca78..d313d2c42b0133d63695c3ca2e583fd9c423802b 100644 (file)
--- 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 f5fec1d8ad36a91c36a26320ca47a612e4c8c504..483ec54dee31c75412808bc5871658a992438562 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -818,7 +818,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);
                }
        }
 
@@ -1830,7 +1830,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
@@ -1906,7 +1906,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);
@@ -2234,7 +2234,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 bc561cf2c65ad17dd5e598675ffcc8ec43845cf4..00990dc47a9eb2b945a6a00e1acbef3d7668dfcc 100644 (file)
--- 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 618825eaaae0c2702ee2edf713c60ac3259178ee..78a4b5211a756cb42995d6dc1168bf68ebc03fe9 100644 (file)
--- a/src/queue.c
+++ b/src/queue.c
@@ -331,9 +331,10 @@ static struct pendconn *pendconn_process_next_strm(struct server *srv, struct pr
 }
 
 /* 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 2b3cc82bed4d175b9d55df985d323862e348564a..0a128a2f1e927b522a296c04c124fba1125a96d5 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -1818,8 +1818,11 @@ const char *server_parse_maxconn_change_request(struct server *sv,
                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 c9c01fce28e4911393ced9b401e8ee9619d555d3..b8da9356413774cf7240392259e9bfc2b82e7af2 100644 (file)
--- 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);
                }
        }