[Asterisk-code-review] chan sip: Destroy peers without holding peers container lock. (asterisk[master])

[Joshua Colp]

Joshua Colp has uploaded a new change for review.

  https://gerrit.asterisk.org/674

Change subject: chan_sip: Destroy peers without holding peers container lock.
......................................................................

chan_sip: Destroy peers without holding peers container lock.

Due to the use of stasis_unsubscribe_and_join in the peer destructor
it is possible for a deadlock to occur when an event callback is
occurring at the same time.

This happens because the peer may be destroyed while holding the
peers container lock. If this occurs the event callback will never
be able to acquire the container lock and the unsubscribe will
never complete.

This change makes it so the peers that have been removed from the
peers container are not destroyed with the container lock held.

ASTERISK-25163 #close

Change-Id: Ic6bf1d9da4310142a4d196c45ddefb99317d9a33
---
M channels/chan_sip.c
1 file changed, 16 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/74/674/1

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 4112e00..b29bcdc 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -3179,10 +3179,24 @@
 
 static void unlink_peers_from_tables(peer_unlink_flag_t flag)
 {
-	ao2_t_callback(peers, OBJ_NODATA | OBJ_UNLINK | OBJ_MULTIPLE,
+	struct ao2_iterator *peers_iter;
+
+	/*
+	 * We must remove the ref outside of the peers container to prevent
+	 * a deadlock condition when unsubscribing from stasis while it is
+	 * invoking a subscription event callback.
+	 */
+	peers_iter = ao2_t_callback(peers, OBJ_UNLINK | OBJ_MULTIPLE,
 		match_and_cleanup_peer_sched, &flag, "initiating callback to remove marked peers");
-	ao2_t_callback(peers_by_ip, OBJ_NODATA | OBJ_UNLINK | OBJ_MULTIPLE,
+	if (peers_iter) {
+		ao2_iterator_destroy(peers_iter);
+	}
+
+	peers_iter = ao2_t_callback(peers_by_ip, OBJ_UNLINK | OBJ_MULTIPLE,
 		match_and_cleanup_peer_sched, &flag, "initiating callback to remove marked peers_by_ip");
+	if (peers_iter) {
+		ao2_iterator_destroy(peers_iter);
+	}
 }
 
 /* \brief Unlink all marked peers from ao2 containers */

-- 
To view, visit https://gerrit.asterisk.org/674
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic6bf1d9da4310142a4d196c45ddefb99317d9a33
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp <jcolp at digium.com>