[asterisk-commits] schmidts: branch schmidts/unleash-the-beast r343798 - in /team/schmidts/unlea...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Tue Nov 8 07:45:25 CST 2011
Author: schmidts
Date: Tue Nov 8 07:45:19 2011
New Revision: 343798
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=343798
Log:
Multiple revisions 343577,343621,343637,343690,343791
........
r343577 | rmudgett | 2011-11-07 19:36:56 +0000 (Mon, 07 Nov 2011) | 18 lines
Fix deadlock if peer is destroyed while sending MWI notice.
A dialog cannot be destroyed by the ao2_callback dialog_needdestroy
because of a deadlock between the dialogs container lock and the RWLOCK of
the events subscription list.
* Create dialogs_to_destroy container to hold dialogs that will be
destroyed.
* Ensure that the event subscription callback will never happen with an
invalid peer pointer by making the event callback removal the first thing
in the peer destructor callback.
(closes issue ASTERISK-18747)
Reported by: Gregory Hinton Nietsky
Review: https://reviewboard.asterisk.org/r/1564/
........
r343621 | kmoore | 2011-11-07 20:27:38 +0000 (Mon, 07 Nov 2011) | 10 lines
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.
(closes issue ASTERISK-18663)
Review: https://reviewboard.asterisk.org/r/1563/
........
r343637 | rmudgett | 2011-11-07 21:13:21 +0000 (Mon, 07 Nov 2011) | 9 lines
Fix __sip_subscribe_mwi_do() incorectly changing dialogs hash key callid.
Changing an object value used as a container key requires removing the
object from the container and reinserting it.
* Created change_callid_pvt() to call instead of build_callid_pvt(). The
change_callid_pvt() will correctly change the dialog callid so the ao2
conainter can explicitly unlink it.
........
r343690 | mnicholson | 2011-11-07 21:40:51 +0000 (Mon, 07 Nov 2011) | 4 lines
respect case changes in peer names on sip reload
ASTERISK-18669
........
r343791 | lmadsen | 2011-11-08 13:26:03 +0000 (Die, 08 Nov 2011) | 4 lines
Fix boo-boo in prep_tarball script.
A hardcoded a branch number was in the prep_tarball which could not work. Changed
it to the variable.
........
Merged revisions 343577,343621,343637,343690,343791 from http://svn.asterisk.org/svn/asterisk/branches/1.8
Modified:
team/schmidts/unleash-the-beast/ (props changed)
team/schmidts/unleash-the-beast/build_tools/prep_tarball
team/schmidts/unleash-the-beast/channels/chan_sip.c
Propchange: team/schmidts/unleash-the-beast/
------------------------------------------------------------------------------
--- svnmerge-integrated (original)
+++ svnmerge-integrated Tue Nov 8 07:45:19 2011
@@ -1,1 +1,1 @@
-/branches/1.8:1-343387
+/branches/1.8:1-343793
Modified: team/schmidts/unleash-the-beast/build_tools/prep_tarball
URL: http://svnview.digium.com/svn/asterisk/team/schmidts/unleash-the-beast/build_tools/prep_tarball?view=diff&rev=343798&r1=343797&r2=343798
==============================================================================
--- team/schmidts/unleash-the-beast/build_tools/prep_tarball (original)
+++ team/schmidts/unleash-the-beast/build_tools/prep_tarball Tue Nov 8 07:45:19 2011
@@ -24,6 +24,6 @@
echo "Extracting HTML Admin Guide"
unzip Asterisk-Admin-Guide-$branch.html.zip
mv AST/ Asterisk-Admin-Guide/
-mv Asterisk-Admin-Guide-1.8.pdf Asterisk-Admin-Guide.pdf
+mv Asterisk-Admin-Guide-$branch.pdf Asterisk-Admin-Guide.pdf
rm -f Asterisk-Admin-Guide-$branch.html.zip
echo "Documentation downloaded. Goodbye!"
Modified: team/schmidts/unleash-the-beast/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/team/schmidts/unleash-the-beast/channels/chan_sip.c?view=diff&rev=343798&r1=343797&r2=343798
==============================================================================
--- team/schmidts/unleash-the-beast/channels/chan_sip.c (original)
+++ team/schmidts/unleash-the-beast/channels/chan_sip.c Tue Nov 8 07:45:19 2011
@@ -1510,6 +1510,7 @@
static int create_addr(struct sip_pvt *dialog, const char *opeer, struct ast_sockaddr *addr, int newdialog, struct ast_sockaddr *remote_address);
static char *generate_random_string(char *buf, size_t size);
static void build_callid_pvt(struct sip_pvt *pvt);
+static void change_callid_pvt(struct sip_pvt *pvt, const char *callid);
static void build_callid_registry(struct sip_registry *reg, const struct ast_sockaddr *ourip, const char *fromdomain);
static void make_our_tag(char *tagbuf, size_t len);
static int add_header(struct sip_request *req, const char *var, const char *value);
@@ -4586,9 +4587,18 @@
static void sip_destroy_peer(struct sip_peer *peer)
{
ast_debug(3, "Destroying SIP peer %s\n", peer->name);
- if (peer->outboundproxy)
+
+ /*
+ * Remove any mailbox event subscriptions for this peer before
+ * we destroy anything. An event subscription callback may be
+ * happening right now.
+ */
+ clear_peer_mailboxes(peer);
+
+ if (peer->outboundproxy) {
ao2_ref(peer->outboundproxy, -1);
- peer->outboundproxy = NULL;
+ peer->outboundproxy = NULL;
+ }
/* Delete it, it needs to disappear */
if (peer->call) {
@@ -4623,7 +4633,6 @@
}
if (peer->dnsmgr)
ast_dnsmgr_release(peer->dnsmgr);
- clear_peer_mailboxes(peer);
if (peer->socket.tcptls_session) {
ao2_ref(peer->socket.tcptls_session, -1);
@@ -5319,15 +5328,20 @@
if (!ast_strlen_zero(peer->fromdomain)) {
ast_string_field_set(dialog, fromdomain, peer->fromdomain);
if (!dialog->initreq.headers) {
- char *c;
+ char *new_callid;
char *tmpcall = ast_strdupa(dialog->callid);
/* this sure looks to me like we are going to change the callid on this dialog!! */
- c = strchr(tmpcall, '@');
- if (c) {
- *c = '\0';
- ao2_t_unlink(dialogs, dialog, "About to change the callid -- remove the old name");
- ast_string_field_build(dialog, callid, "%s@%s", tmpcall, peer->fromdomain);
- ao2_t_link(dialogs, dialog, "New dialog callid -- inserted back into table");
+ new_callid = strchr(tmpcall, '@');
+ if (new_callid) {
+ int callid_size;
+
+ *new_callid = '\0';
+
+ /* Change the dialog callid. */
+ callid_size = strlen(tmpcall) + strlen(peer->fromdomain) + 2;
+ new_callid = alloca(callid_size);
+ snprintf(new_callid, callid_size, "%s@%s", tmpcall, peer->fromdomain);
+ change_callid_pvt(dialog, new_callid);
}
}
}
@@ -7460,15 +7474,60 @@
return buf;
}
-/*! \brief Build SIP Call-ID value for a non-REGISTER transaction */
+/*!
+ * \brief Build SIP Call-ID value for a non-REGISTER transaction
+ *
+ * \note The passed in pvt must not be in a dialogs container
+ * since this function changes the hash key used by the
+ * container.
+ */
static void build_callid_pvt(struct sip_pvt *pvt)
{
char buf[33];
-
const char *host = S_OR(pvt->fromdomain, ast_sockaddr_stringify_remote(&pvt->ourip));
-
+
ast_string_field_build(pvt, callid, "%s@%s", generate_random_string(buf, sizeof(buf)), host);
-
+}
+
+/*! \brief Unlink the given object from the container and return TRUE if it was in the container. */
+#define CONTAINER_UNLINK(container, obj, tag) \
+ ({ \
+ int found = 0; \
+ typeof((obj)) __removed_obj; \
+ __removed_obj = ao2_t_callback((container), \
+ OBJ_UNLINK | OBJ_POINTER, ao2_match_by_addr, (obj), (tag)); \
+ if (__removed_obj) { \
+ ao2_ref(__removed_obj, -1); \
+ found = 1; \
+ } \
+ found; \
+ })
+
+/*!
+ * \internal
+ * \brief Safely change the callid of the given SIP dialog.
+ *
+ * \param pvt SIP private structure to change callid
+ * \param callid Specified new callid to use. NULL if generate new callid.
+ *
+ * \return Nothing
+ */
+static void change_callid_pvt(struct sip_pvt *pvt, const char *callid)
+{
+ int in_dialog_container;
+
+ ao2_lock(dialogs);
+ in_dialog_container = CONTAINER_UNLINK(dialogs, pvt,
+ "About to change the callid -- remove the old name");
+ if (callid) {
+ ast_string_field_set(pvt, callid, callid);
+ } else {
+ build_callid_pvt(pvt);
+ }
+ if (in_dialog_container) {
+ ao2_t_link(dialogs, pvt, "New dialog callid -- inserted back into table");
+ }
+ ao2_unlock(dialogs);
}
/*! \brief Build SIP Call-ID value for a REGISTER transaction */
@@ -12182,7 +12241,10 @@
ast_sip_ouraddrfor(&mwi->call->sa, &mwi->call->ourip, mwi->call);
build_contact(mwi->call);
build_via(mwi->call);
- build_callid_pvt(mwi->call);
+
+ /* Change the dialog callid. */
+ change_callid_pvt(mwi->call, NULL);
+
ast_set_flag(&mwi->call->flags[0], SIP_OUTGOING);
/* Associate the call with us */
@@ -24369,7 +24431,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 */
@@ -24445,7 +24509,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) {
@@ -25156,36 +25225,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;
}
@@ -25199,20 +25281,23 @@
dialog_unlink_all(p);
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 */
ast_sip_ouraddrfor(&p->sa, &p->ourip, p);
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_t_link(dialogs, p, "Linking in under new name");
+ ao2_unlock(peer);
+
+ /* Change the dialog callid. */
+ change_callid_pvt(p, NULL);
+
/* Destroy this session after 32 secs */
sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
}
@@ -25224,10 +25309,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;
}
@@ -25833,9 +25918,9 @@
/* Recalculate our side, and recalculate Call ID */
ast_sip_ouraddrfor(&p->sa, &p->ourip, p);
build_via(p);
- ao2_t_unlink(dialogs, p, "About to change the callid -- remove the old name");
- build_callid_pvt(p);
- ao2_t_link(dialogs, p, "Linking in under new name");
+
+ /* Change the dialog callid. */
+ change_callid_pvt(p, NULL);
AST_SCHED_DEL_UNREF(sched, peer->pokeexpire,
unref_peer(peer, "removing poke peer ref"));
@@ -26131,10 +26216,10 @@
/* Recalculate our side, and recalculate Call ID */
ast_sip_ouraddrfor(&p->sa, &p->ourip, p);
build_via(p);
- ao2_t_unlink(dialogs, p, "About to change the callid -- remove the old name");
- build_callid_pvt(p);
- ao2_t_link(dialogs, p, "Linking in under new name");
-
+
+ /* Change the dialog callid. */
+ change_callid_pvt(p, NULL);
+
/* We have an extension to call, don't use the full contact here */
/* This to enable dialing registered peers with extension dialling,
like SIP/peername/extension
@@ -26797,8 +26882,9 @@
set_peer_defaults(peer); /* Set peer defaults */
peer->type = 0;
}
- if (!found && name)
- ast_copy_string(peer->name, name, sizeof(peer->name));
+
+ /* in case the case of the peer name has changed, update the name */
+ ast_copy_string(peer->name, name, sizeof(peer->name));
/* If we have channel variables, remove them (reload) */
if (peer->chanvars) {
@@ -29746,7 +29832,7 @@
dialogs_needdestroy = ao2_t_container_alloc(HASH_DIALOG_SIZE, dialog_hash_cb, dialog_cmp_cb, "allocate dialogs_needdestroy");
dialogs_rtpcheck = ao2_t_container_alloc(HASH_DIALOG_SIZE, dialog_hash_cb, dialog_cmp_cb, "allocate dialogs for rtpchecks");
threadt = ao2_t_container_alloc(HASH_DIALOG_SIZE, threadt_hash_cb, threadt_cmp_cb, "allocate threadt table");
- if (!peers || !peers_by_ip || !dialogs || !threadt) {
+ if (!peers || !peers_by_ip || !dialogs || !dialogs_needdestroy || !dialogs_rtpcheck || !threadt) {
ast_log(LOG_ERROR, "Unable to create primary SIP container(s)\n");
return AST_MODULE_LOAD_FAILURE;
}
More information about the asterisk-commits
mailing list