[asterisk-commits] bebuild: tag 1.8.8.0-rc3 r344270 - in /tags/1.8.8.0-rc3: ./ channels/chan_sip.c
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Wed Nov 9 14:50:37 CST 2011
Author: bebuild
Date: Wed Nov 9 14:50:33 2011
New Revision: 344270
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=344270
Log:
Prevent BLF subscriptions from causing deadlocks
Fix a locking inversion in sip_send_mwi_to_peer that was causing deadlocks.
This function now requires that both the peer and associated pvt be unlocked
before it is called for cases where peer and peer->mwipvt form a circular
reference.
(issue ASTERISK-18663)
Modified:
tags/1.8.8.0-rc3/ (props changed)
tags/1.8.8.0-rc3/channels/chan_sip.c
Propchange: tags/1.8.8.0-rc3/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Nov 9 14:50:33 2011
@@ -1,1 +1,1 @@
-/branches/1.8:339719,339779,340878,341088
+/branches/1.8:339719,339779,340878,341088,343621
Modified: tags/1.8.8.0-rc3/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/tags/1.8.8.0-rc3/channels/chan_sip.c?view=diff&rev=344270&r1=344269&r2=344270
==============================================================================
--- tags/1.8.8.0-rc3/channels/chan_sip.c (original)
+++ tags/1.8.8.0-rc3/channels/chan_sip.c Wed Nov 9 14:50:33 2011
@@ -24208,7 +24208,9 @@
p->subscribed = MWI_NOTIFICATION;
if (ast_test_flag(&authpeer->flags[1], SIP_PAGE2_SUBSCRIBEMWIONLY)) {
+ ao2_unlock(p);
add_peer_mwi_subs(authpeer);
+ ao2_lock(p);
}
if (authpeer->mwipvt && authpeer->mwipvt != p) { /* Destroy old PVT if this is a new one */
/* We only allow one subscription per peer */
@@ -24284,7 +24286,12 @@
ast_set_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED);
transmit_response(p, "200 OK", req);
if (p->relatedpeer) { /* Send first notification */
- sip_send_mwi_to_peer(p->relatedpeer, 0);
+ struct sip_peer *peer = p->relatedpeer;
+ ref_peer(peer, "ensure a peer ref is held during MWI sending");
+ ao2_unlock(p);
+ sip_send_mwi_to_peer(peer, 0);
+ ao2_lock(p);
+ unref_peer(peer, "release a peer ref now that MWI is sent");
}
} else if (p->subscribed != CALL_COMPLETION) {
if ((firststate = ast_extension_state(NULL, p->context, p->exten)) < 0) {
@@ -24995,36 +25002,49 @@
return in_cache;
}
-/*! \brief Send message waiting indication to alert peer that they've got voicemail */
+/*! \brief Send message waiting indication to alert peer that they've got voicemail
+ * \note Both peer and associated sip_pvt must be unlocked prior to calling this function
+*/
static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only)
{
/* Called with peerl lock, but releases it */
struct sip_pvt *p;
int newmsgs = 0, oldmsgs = 0;
-
- if (ast_test_flag((&peer->flags[1]), SIP_PAGE2_SUBSCRIBEMWIONLY) && !peer->mwipvt)
+ const char *vmexten;
+
+ ao2_lock(peer);
+
+ vmexten = ast_strdupa(peer->vmexten);
+
+ if (ast_test_flag((&peer->flags[1]), SIP_PAGE2_SUBSCRIBEMWIONLY) && !peer->mwipvt) {
+ ao2_unlock(peer);
return 0;
+ }
/* Do we have an IP address? If not, skip this peer */
- if (ast_sockaddr_isnull(&peer->addr) && ast_sockaddr_isnull(&peer->defaddr))
+ if (ast_sockaddr_isnull(&peer->addr) && ast_sockaddr_isnull(&peer->defaddr)) {
+ ao2_unlock(peer);
return 0;
+ }
/* Attempt to use cached mwi to get message counts. */
if (!get_cached_mwi(peer, &newmsgs, &oldmsgs) && !cache_only) {
/* Fall back to manually checking the mailbox if not cache_only and get_cached_mwi failed */
struct ast_str *mailbox_str = ast_str_alloca(512);
peer_mailboxes_to_str(&mailbox_str, peer);
+ ao2_unlock(peer);
ast_app_inboxcount(mailbox_str->str, &newmsgs, &oldmsgs);
- }
- ao2_lock(peer);
+ ao2_lock(peer);
+ }
if (peer->mwipvt) {
/* Base message on subscription */
- p = dialog_ref(peer->mwipvt, "sip_send_mwi_to_peer: Setting dialog ptr p from peer->mwipvt-- should this be done?");
+ p = dialog_ref(peer->mwipvt, "sip_send_mwi_to_peer: Setting dialog ptr p from peer->mwipvt");
+ ao2_unlock(peer);
} else {
+ ao2_unlock(peer);
/* Build temporary dialog for this message */
if (!(p = sip_alloc(NULL, NULL, 0, SIP_NOTIFY, NULL))) {
- ao2_unlock(peer);
return -1;
}
@@ -25038,7 +25058,6 @@
dialog_unlink_all(p, TRUE, TRUE);
dialog_unref(p, "unref dialog p just created via sip_alloc");
/* sip_destroy(p); */
- ao2_unlock(peer);
return 0;
}
/* Recalculate our side, and recalculate Call ID */
@@ -25046,11 +25065,15 @@
build_via(p);
ao2_t_unlink(dialogs, p, "About to change the callid -- remove the old name");
build_callid_pvt(p);
+
+ ao2_lock(peer);
if (!ast_strlen_zero(peer->mwi_from)) {
ast_string_field_set(p, mwi_from, peer->mwi_from);
} else if (!ast_strlen_zero(default_mwi_from)) {
ast_string_field_set(p, mwi_from, default_mwi_from);
}
+ ao2_unlock(peer);
+
ao2_t_link(dialogs, p, "Linking in under new name");
/* Destroy this session after 32 secs */
sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
@@ -25063,10 +25086,10 @@
/* Send MWI */
ast_set_flag(&p->flags[0], SIP_OUTGOING);
/* the following will decrement the refcount on p as it finishes */
- transmit_notify_with_mwi(p, newmsgs, oldmsgs, peer->vmexten);
+ transmit_notify_with_mwi(p, newmsgs, oldmsgs, vmexten);
sip_pvt_unlock(p);
dialog_unref(p, "unref dialog ptr p just before it goes out of scope at the end of sip_send_mwi_to_peer.");
- ao2_unlock(peer);
+
return 0;
}
More information about the asterisk-commits
mailing list