[Asterisk-code-review] chan sip: Prevent deadlock when issuing "sip show channels" (asterisk[master])

[George Joseph]

George Joseph has uploaded a new change for review.

  https://gerrit.asterisk.org/3275

Change subject: chan_sip: Prevent deadlock when issuing "sip show channels"
......................................................................

chan_sip: Prevent deadlock when issuing "sip show channels"

sip_chow_channels locks the dialogs container first then locks each
sip_pvt so it can spit out the details.  The rest of sip dialog
processing locks the sip_pvt first then locks the dialogs container
if it needs to.  Both lock in the order they need but deadlocks can
result.  To fix, show_channels_cb now does a trylock on the sip_pvt
instead of a lock and if the lock can't be obtained after 10 retries
over 1ms, the callback just returns and the channel is skipped in
the oputput.

ASTERISK-23013 #close
patches:
  deadlock_fix.diff (modified) submitted by Ben Smithurst (license 6529)

Change-Id: Id9980419909e811f89484950ed46ef117b9eb990
---
M channels/chan_sip.c
1 file changed, 18 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/75/3275/1

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index b2522b6..1acc6cc 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -21725,6 +21725,8 @@
 #define FORMAT3 "%-15.15s  %-15.15s  %-15.15s  %-15.15s  %-13.13s  %-15.15s %-10.10s %-6.6s\n"
 #define FORMAT2 "%-15.15s  %-15.15s  %-15.15s  %-15.15s  %-7.7s  %-15.15s %-10.10s %-10.10s\n"
 #define FORMAT  "%-15.15s  %-15.15s  %-15.15s  %-15.15s  %-3.3s %-3.3s  %-15.15s %-10.10s %-10.10s\n"
+#define SHOW_CHANNELS_RETRIES 10
+#define SHOW_CHANNELS_WAIT 100
 
 /*! \brief callback for show channel|subscription */
 static int show_channels_cb(void *__cur, void *__arg, int flags)
@@ -21732,8 +21734,23 @@
 	struct sip_pvt *cur = __cur;
 	struct __show_chan_arg *arg = __arg;
 	const struct ast_sockaddr *dst;
+	int tries = 0;
 
-	sip_pvt_lock(cur);
+	/*
+	 * sip_show_channels locks dialogs and sip_pvt in the reverse order than normal
+	 * dialog processing.  In order to prevent a deadlock, we have to trylock the sip_pvt
+	 * a few times and return if we can't get the lock.
+	 */
+	for (tries = 0; tries < SHOW_CHANNELS_RETRIES; tries++) {
+		if (sip_pvt_trylock(cur) == 0) {
+			goto locked;
+		}
+		usleep(SHOW_CHANNELS_WAIT);
+	}
+
+	return 0;
+
+locked:
 	dst = sip_real_dst(cur);
 
 	/* XXX indentation preserved to reduce diff. Will be fixed later */

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id9980419909e811f89484950ed46ef117b9eb990
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <gjoseph at digium.com>