From f648767a4eb0c24616eeb6da34d28681ff1dc2c9 Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Wed, 23 Nov 2022 18:32:26 +0100 Subject: [PATCH] MINOR: peers: unused code path in process_peer_sync In process_peer_sync: a check was performed to know whether the peers section handler should kill itself if the corresponding proxy was not started on the current process. This logic was initially implemented in early 1.6 development to prevent some issues when peers where used in conjunction with nbproc > 1: f83d3fe00a MEDIUM: init: stop any peers section not bound to the correct process 46dc1ca MEDIUM: peers: unregister peers that were never started But later in 1.6 dev, a new commit has been introduced: 47c8c029db MEDIUM: init: completely deallocate unused peers With the latter, the check implemented in 46dc1ca ("MEDIUM: peers: unregister peers that were never started") will never succeed: it is dead code. Since nbproc support has been dropped in 2.5, things have changed a bit: f83d3fe00a logic was moved in mworker_cleanlisteners, but as in 46dc1ca : peers task is safely destroyed before peers_fe is set to NULL. Conversely, peers_fe is first set by init_peers_frontend() before peers task is scheduled by peers_init_sync() in check_config_validity(). Again, it is safe to say that we will never reach !peers->peers_fe in process_peer_sync(): this self-killing mechanism is not relevant anymore. -- To cut a long story short: I stumbled on this while tracking down current signal api usage. This led me to a signal_unregister_handler() call performed in the aforementionned dead code. To me this code was potentially unsafe because signal_unregister_handler() is not thread safe and here it was used within a task initialized via task_new_anywhere(). So I decided to check how bad this could be (ie: conditions to be met for this code to run).. and here we are. --- src/peers.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/peers.c b/src/peers.c index aa0ab9734..8fc84bc68 100644 --- a/src/peers.c +++ b/src/peers.c @@ -3304,14 +3304,6 @@ struct task *process_peer_sync(struct task * task, void *context, unsigned int s task->expire = TICK_ETERNITY; - if (!peers->peers_fe) { - /* this one was never started, kill it */ - signal_unregister_handler(peers->sighandler); - task_destroy(peers->sync_task); - peers->sync_task = NULL; - return NULL; - } - /* Acquire lock for all peers of the section */ for (ps = peers->remote; ps; ps = ps->next) HA_SPIN_LOCK(PEER_LOCK, &ps->lock);