]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
chan_sip.c: Fix reinviteid deadlock potential. 13/2413/2
authorRichard Mudgett <rmudgett@digium.com>
Wed, 9 Mar 2016 22:34:53 +0000 (16:34 -0600)
committerRichard Mudgett <rmudgett@digium.com>
Wed, 16 Mar 2016 19:44:51 +0000 (14:44 -0500)
This patch is part of a series to resolve deadlocks in chan_sip.c.

Stopping a scheduled event can result in a deadlock if the scheduled event
is running when you try to stop the event.  If you hold a lock needed by
the scheduled event while trying to stop the scheduled event then a
deadlock can happen.  The general strategy for resolving the deadlock
potential is to push the actual starting and stopping of the scheduled
events off onto the scheduler/do_monitor() thread by scheduling an
immediate one shot scheduled event.  Some restructuring may be needed
because the code may assume that the start/stop of the scheduled events is
immediate.

ASTERISK-25023

Change-Id: I9c11b9d597468f63916c99e1dabff9f4a46f84c1

channels/chan_sip.c

index 17ecba22db78abd3f5a10817b542f4385f842f1d..fe06085380db72ba469f1f6312c8a46a28dc6385 100644 (file)
@@ -7046,21 +7046,44 @@ const char *hangup_cause2sip(int cause)
        return 0;
 }
 
+/* Run by the sched thread. */
 static int reinvite_timeout(const void *data)
 {
        struct sip_pvt *dialog = (struct sip_pvt *) data;
-       struct ast_channel *owner = sip_pvt_lock_full(dialog);
+       struct ast_channel *owner;
+
+       owner = sip_pvt_lock_full(dialog);
        dialog->reinviteid = -1;
        check_pendings(dialog);
        if (owner) {
                ast_channel_unlock(owner);
                ast_channel_unref(owner);
        }
-       ao2_unlock(dialog);
-       dialog_unref(dialog, "unref for reinvite timeout");
+       sip_pvt_unlock(dialog);
+       dialog_unref(dialog, "reinviteid complete");
+       return 0;
+}
+
+/* Run by the sched thread. */
+static int __stop_reinviteid(const void *data)
+{
+       struct sip_pvt *pvt = (void *) data;
+
+       AST_SCHED_DEL_UNREF(sched, pvt->reinviteid,
+               dialog_unref(pvt, "Stop scheduled reinviteid"));
+       dialog_unref(pvt, "Stop reinviteid action");
        return 0;
 }
 
+static void stop_reinviteid(struct sip_pvt *pvt)
+{
+       dialog_ref(pvt, "Stop reinviteid action");
+       if (ast_sched_add(sched, 0, __stop_reinviteid, pvt) < 0) {
+               /* Uh Oh.  Expect bad behavior. */
+               dialog_unref(pvt, "Failed to schedule stop reinviteid action");
+       }
+}
+
 /*! \brief  sip_hangup: Hangup SIP call
  * Part of PBX interface, called from ast_hangup */
 static int sip_hangup(struct ast_channel *ast)
@@ -7252,7 +7275,12 @@ static int sip_hangup(struct ast_channel *ast)
                                 * So, just in case, check for pending actions after a bit of time to trigger the pending
                                 * bye that we are setting above */
                                if (p->ongoing_reinvite && p->reinviteid < 0) {
-                                       p->reinviteid = ast_sched_add(sched, 32 * p->timer_t1, reinvite_timeout, dialog_ref(p, "ref for reinvite_timeout"));
+                                       p->reinviteid = ast_sched_add(sched, 32 * p->timer_t1,
+                                               reinvite_timeout, dialog_ref(p, "Schedule reinviteid"));
+                                       if (p->reinviteid < 0) {
+                                               /* Uh Oh.  Expect bad behavior. */
+                                               dialog_unref(p, "Failed to schedule reinviteid");
+                                       }
                                }
                        }
                }
@@ -22983,13 +23011,14 @@ static void check_pendings(struct sip_pvt *p)
                if (p->reinviteid > -1) {
                        /* Outstanding p->reinviteid timeout, so wait... */
                        return;
-               } else if (p->invitestate == INV_PROCEEDING || p->invitestate == INV_EARLY_MEDIA) {
+               }
+               if (p->invitestate == INV_PROCEEDING || p->invitestate == INV_EARLY_MEDIA) {
                        /* if we can't BYE, then this is really a pending CANCEL */
                        p->invitestate = INV_CANCELLED;
                        transmit_request(p, SIP_CANCEL, p->lastinvite, XMIT_RELIABLE, FALSE);
                        /* If the cancel occurred on an initial invite, cancel the pending BYE */
                        if (!ast_test_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED)) {
-                               ast_clear_flag(&p->flags[0], SIP_PENDINGBYE);
+                               ast_clear_flag(&p->flags[0], SIP_PENDINGBYE | SIP_NEEDREINVITE);
                        }
                        /* Actually don't destroy us yet, wait for the 487 on our original
                           INVITE, but do set an autodestruct just in case we never get it. */
@@ -23005,7 +23034,7 @@ static void check_pendings(struct sip_pvt *p)
                        }
                        /* Perhaps there is an SD change INVITE outstanding */
                        transmit_request_with_auth(p, SIP_BYE, 0, XMIT_RELIABLE, TRUE);
-                       ast_clear_flag(&p->flags[0], SIP_PENDINGBYE);
+                       ast_clear_flag(&p->flags[0], SIP_PENDINGBYE | SIP_NEEDREINVITE);
                }
                sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
        } else if (ast_test_flag(&p->flags[0], SIP_NEEDREINVITE)) {
@@ -23301,9 +23330,7 @@ static void handle_response_invite(struct sip_pvt *p, int resp, const char *rest
 
        if ((resp >= 200 && reinvite)) {
                p->ongoing_reinvite = 0;
-               if (p->reinviteid > -1) {
-                       AST_SCHED_DEL_UNREF(sched, p->reinviteid, dialog_unref(p, "unref dialog for reinvite timeout because of a final response"));
-               }
+               stop_reinviteid(p);
        }
 
        /* Final response, clear out pending invite */