BUG/MEDIUM: protocols: add a global lock for the init/deinit stuff
Dragan Dosen found that the listeners lock is not sufficient to protect
the listeners list when proxies are stopping because the listeners are
also unlinked from the protocol list, and under certain situations like
bombing with soft-stop signals or shutting down many frontends in parallel
from multiple CLI connections, it could be possible to provoke multiple
instances of delete_listener() to be called in parallel for different
listeners, thus corrupting the protocol lists.
Such operations are pretty rare, they are performed once per proxy upon
startup and once per proxy on shut down. Thus there is no point trying
to optimize anything and we can use a global lock to protect the protocol
lists during these manipulations.
This fix (or a variant) will have to be backported as far as 1.8.
(cherry picked from commit daacf3664506d56a1f3b050ccba504886a18b12a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/listener.c b/src/listener.c
index 40a774e..b5fe2ac 100644
--- a/src/listener.c
+++ b/src/listener.c
@@ -433,6 +433,9 @@
* used as a protocol's generic enable_all() primitive, for use after the
* fork(). It puts the listeners into LI_READY or LI_FULL states depending on
* their number of connections. It always returns ERR_NONE.
+ *
+ * Must be called with proto_lock held.
+ *
*/
int enable_all_listeners(struct protocol *proto)
{
@@ -447,6 +450,9 @@
* the polling lists when they are in the LI_READY or LI_FULL states. It is
* intended to be used as a protocol's generic disable_all() primitive. It puts
* the listeners into LI_LISTEN, and always returns ERR_NONE.
+ *
+ * Must be called with proto_lock held.
+ *
*/
int disable_all_listeners(struct protocol *proto)
{
@@ -516,6 +522,9 @@
/* This function closes all listening sockets bound to the protocol <proto>,
* and the listeners end in LI_ASSIGNED state if they were higher. It does not
* detach them from the protocol. It always returns ERR_NONE.
+ *
+ * Must be called with proto_lock held.
+ *
*/
int unbind_all_listeners(struct protocol *proto)
{
@@ -580,14 +589,19 @@
* number of listeners is updated, as well as the global number of listeners
* and jobs. Note that the listener must have previously been unbound. This
* is the generic function to use to remove a listener.
+ *
+ * Will grab the proto_lock.
+ *
*/
void delete_listener(struct listener *listener)
{
HA_SPIN_LOCK(LISTENER_LOCK, &listener->lock);
if (listener->state == LI_ASSIGNED) {
listener->state = LI_INIT;
+ HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
LIST_DEL(&listener->proto_list);
listener->proto->nb_listeners--;
+ HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
_HA_ATOMIC_SUB(&jobs, 1);
_HA_ATOMIC_SUB(&listeners, 1);
}