diff options
author | Richard Mudgett <rmudgett@digium.com> | 2011-11-07 19:54:09 +0000 |
---|---|---|
committer | Richard Mudgett <rmudgett@digium.com> | 2011-11-07 19:54:09 +0000 |
commit | 7a5f6684f05e03c51b5d004156dba9df2711dd50 (patch) | |
tree | 5615007199f6562a742de6f54533eb666eb17014 /channels/chan_sip.c | |
parent | f3c03ac1ecd9f8dec32edeaf9b1c3efd6a7e9e50 (diff) |
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.
NOTE: This particular deadlock will not happen with Asterisk 10, but some
of the changes still apply.
(closes issue ASTERISK-18747)
Reported by: Gregory Hinton Nietsky
Review: https://reviewboard.asterisk.org/r/1564/
........
Merged revisions 343577 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........
Merged revisions 343578 from http://svn.asterisk.org/svn/asterisk/branches/10
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@343579 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Diffstat (limited to 'channels/chan_sip.c')
-rw-r--r-- | channels/chan_sip.c | 25 |
1 files changed, 19 insertions, 6 deletions
diff --git a/channels/chan_sip.c b/channels/chan_sip.c index e7b678b9c..c55052385 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -2956,6 +2956,13 @@ static void *dialog_unlink_rtpcheck(struct sip_pvt *dialog) return NULL; } +/*! + * \brief Unlink a dialog from the dialogs container, as well as any other places + * that it may be currently stored. + * + * \note A reference to the dialog must be held before calling this function, and this + * function does not release that reference. + */ void dialog_unlink_all(struct sip_pvt *dialog) { struct sip_pkt *cp; @@ -4589,9 +4596,18 @@ static void sip_destroy_peer_fn(void *peer) 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) { @@ -4625,7 +4641,6 @@ static void sip_destroy_peer(struct sip_peer *peer) } if (peer->dnsmgr) ast_dnsmgr_release(peer->dnsmgr); - clear_peer_mailboxes(peer); if (peer->socket.tcptls_session) { ao2_ref(peer->socket.tcptls_session, -1); @@ -17045,14 +17060,12 @@ static int dialog_checkrtp_cb(void *dialogobj, void *arg, int flags) * \brief Match dialogs that need to be destroyed * * \details This is used with ao2_callback to unlink/delete all dialogs that - * are marked needdestroy. It will return CMP_MATCH for candidates, and they - * will be unlinked. + * are marked needdestroy. * * \todo Re-work this to improve efficiency. Currently, this function is called * on _every_ dialog after processing _every_ incoming SIP/UDP packet, or * potentially even more often when the scheduler has entries to run. */ - static int dialog_needdestroy(void *dialogobj, void *arg, int flags) { struct sip_pvt *dialog = dialogobj; |