Skip to content

Commit f521c2c

Browse files
committed
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.
1 parent ee1f052 commit f521c2c

1 file changed

Lines changed: 17 additions & 9 deletions

File tree

src/server.c

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3218,13 +3218,18 @@ void srv_free_params(struct server *srv)
32183218
struct server *srv_drop(struct server *srv)
32193219
{
32203220
struct server *next = NULL;
3221+
struct proxy *px = NULL;
32213222
int i __maybe_unused;
32223223

32233224
if (!srv)
32243225
goto end;
32253226

32263227
next = srv->next;
32273228

3229+
/* If srv was deleted, a proxy refcount must be dropped. */
3230+
if (srv->flags & SRV_F_DELETED)
3231+
px = srv->proxy;
3232+
32283233
/* For dynamic servers, decrement the reference counter. Only free the
32293234
* server when reaching zero.
32303235
*/
@@ -3264,6 +3269,8 @@ struct server *srv_drop(struct server *srv)
32643269

32653270
srv_free(&srv);
32663271

3272+
proxy_drop(px);
3273+
32673274
end:
32683275
return next;
32693276
}
@@ -6527,6 +6534,16 @@ static int cli_parse_delete_server(char **args, char *payload, struct appctx *ap
65276534
*/
65286535
srv_detach(srv);
65296536

6537+
/* Mark the server as being deleted (ie removed from its proxy list)
6538+
* but not yet purged from memory. Any module still referencing this
6539+
* server must manipulate it with precaution and are expected to
6540+
* release its refcount as soon as possible.
6541+
*/
6542+
srv->flags |= SRV_F_DELETED;
6543+
6544+
/* Inc proxy refcount until the server is finally freed. */
6545+
proxy_take(srv->proxy);
6546+
65306547
/* remove srv from addr_node tree */
65316548
ceb32_item_delete(&be->conf.used_server_id, conf.puid_node, puid, srv);
65326549
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
65356552
/* remove srv from idle_node tree for idle conn cleanup */
65366553
eb32_delete(&srv->idle_node);
65376554

6538-
/* flag the server as deleted
6539-
* (despite the server being removed from primary server list,
6540-
* one could still access the server data from a valid ptr)
6541-
* Deleted flag helps detecting when a server is in transient removal
6542-
* state.
6543-
* ie: removed from the list but not yet freed/purged from memory.
6544-
*/
6545-
srv->flags |= SRV_F_DELETED;
6546-
65476555
/* set LSB bit (odd bit) for reuse_cnt */
65486556
srv_id_reuse_cnt |= 1;
65496557

0 commit comments

Comments
 (0)