BUG/MINOR: peers: forget the appctx when the peer session init fails

peer_session_create() assigns the new appctx to peer->appctx and accounts
it in the per-thread applet counters, then the session is initialized by
peer_session_init() when the appctx first runs. If this initialization
fails (e.g. memory allocation failure), the appctx is freed by
appctx_free_on_early_error() without the release callback being called, so
peer->appctx keeps pointing to freed memory: the sync task then never
tries to reconnect this peer (it believes a session exists), a later
incoming connection from this peer or a soft-stop would call
peer_session_forceshutdown() on the freed appctx, and the applet counter
remains incremented.

Let's make peer_session_init() detach the appctx from the peer on error,
release the applet counter, and wake the sync task up so that a new
connection is attempted once the reconnect timer expires.

This can be backported to all stable versions where peer_session_init()
exists (2.6 and above).

diff --git a/src/peers.c b/src/peers.c
index c09e585533e06fc4f03fe3ff0079321185902aa8..b13c8ca7f1755a83b3701f5d75c104eb7c2804ee 100644 (file)
--- a/src/peers.c
+++ b/src/peers.c
@@ -1158,6 +1158,17 @@ static int peer_session_init(struct appctx *appctx)
  out_free_addr:
        sockaddr_free(&addr);
  out_error:
+       /* the appctx will be freed without calling the release callback, so
+        * the peer must forget it, otherwise it would keep a dangling pointer
+        * and would never try to reconnect.
+        */
+       HA_SPIN_LOCK(PEER_LOCK, &peer->lock);
+       if (peer->appctx == appctx) {
+               peer->appctx = NULL;
+               HA_ATOMIC_DEC(&peer->peers->applet_count[tid]);
+       }
+       HA_SPIN_UNLOCK(PEER_LOCK, &peer->lock);
+       task_wakeup(peer->peers->sync_task, TASK_WOKEN_MSG);
        TRACE_ERROR("peer session init failed", PEERS_EV_SESS_NEW|PEERS_EV_SESS_END|PEERS_EV_SESS_ERR, NULL, peer);
        return -1;
 }