MINOR: server: take proxy refcount when deleting a server

When a server is deleted via "del server", increment refcount of its
parent backend. This is necessary as the server is not referenced
anymore in the backend, but can still access it via its own <proxy>
member. Thus, backend removal must not happen until the complete purge
of the server.

The proxy refcount is released in srv_drop() if the flag SRV_F_DELETED
is set, which indicates that "del server" was used. This operation is
performed after the complete release of the server instance to ensure no
access will be performed on the proxy via itself. The refcount must not
be decremented if a server is freed without "del server" invokation.

Another solution could be for servers to always increment the refcount.
However, for now in haproxy refcount usage is limited, so the current
approach is preferred. It should also ensure that if the refcount is
still incremented, it may indicate that some servers are not completely
purged themselves.

Note that this patch may cause issues if "del backend" are used in
parallel with LUA scripts referencing servers. Currently, any servers
referenced by LUA must be released by its garbage collector to ensure it
can be finally freed. However, it appeas that in some case the gc does
not run for several minutes. At least this has been observed with Lua
version 5.4.8. In the end, this will result in indefinitely blocking of
"del backend" commands.

diff --git a/src/server.c b/src/server.c
index 4718257a12b38e96547b15cf157aec7957e78e83..211a6cf522c09795fabb86b7a238aec8a1134409 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -3218,6 +3218,7 @@ void srv_free_params(struct server *srv)
 struct server *srv_drop(struct server *srv)
 {
        struct server *next = NULL;
+       struct proxy *px = NULL;
        int i __maybe_unused;
 
        if (!srv)
@@ -3225,6 +3226,10 @@ struct server *srv_drop(struct server *srv)
 
        next = srv->next;
 
+       /* If srv was deleted, a proxy refcount must be dropped. */
+       if (srv->flags & SRV_F_DELETED)
+               px = srv->proxy;
+
        /* For dynamic servers, decrement the reference counter. Only free the
         * server when reaching zero.
         */
@@ -3264,6 +3269,8 @@ struct server *srv_drop(struct server *srv)
 
        srv_free(&srv);
 
+       proxy_drop(px);
+
  end:
        return next;
 }
@@ -6527,6 +6534,16 @@ static int cli_parse_delete_server(char **args, char *payload, struct appctx *ap
         */
        srv_detach(srv);
 
+       /* Mark the server as being deleted (ie removed from its proxy list)
+        * but not yet purged from memory. Any module still referencing this
+        * server must manipulate it with precaution and are expected to
+        * release its refcount as soon as possible.
+        */
+       srv->flags |= SRV_F_DELETED;
+
+       /* Inc proxy refcount until the server is finally freed. */
+       proxy_take(srv->proxy);
+
        /* remove srv from addr_node tree */
        ceb32_item_delete(&be->conf.used_server_id, conf.puid_node, puid, srv);
        cebis_item_delete(&be->conf.used_server_name, conf.name_node, id, srv);
@@ -6535,15 +6552,6 @@ static int cli_parse_delete_server(char **args, char *payload, struct appctx *ap
        /* remove srv from idle_node tree for idle conn cleanup */
        eb32_delete(&srv->idle_node);
 
-       /* flag the server as deleted
-        * (despite the server being removed from primary server list,
-        * one could still access the server data from a valid ptr)
-        * Deleted flag helps detecting when a server is in transient removal
-        * state.
-        * ie: removed from the list but not yet freed/purged from memory.
-        */
-       srv->flags |= SRV_F_DELETED;
-
        /* set LSB bit (odd bit) for reuse_cnt */
        srv_id_reuse_cnt |= 1;