[Asterisk-code-review] channels/chan pjsip: fix deadlock in response retransmissions (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/9477 )

Change subject: channels/chan_pjsip: fix deadlock in response retransmissions
......................................................................


Patch Set 8: Code-Review-1

> > The timer must be stopped before the call to pjsip_inv_answer as
 > > pjsip_inv_answer is what is modifying the tdata (and thus causing
 > > the re transmission timer to get in an infinite loop)
 > 
 > To be more precise pjsip_inv_answer calls pjsip_dlg_modify_response
 > on the inv->last_answer which is the same tdata object as the
 > tsx->last_tx being used by the re-transmission timer.  so
 > pjsip_inv_answer can not be called safely while any re-transmission
 > timer is active

That makes sense.  However, it would make more sense for pjsip_inv_answer() to cancel the timer itself since there are three places in chan_pjsip.c that call pjsip_inv_answer() and the deadlock can happen for any response after we send 100 trying.

In the pjsip/src/pjsip/sip_transaction.c:tsx_on_state_proceeding_uas() function it calls:

	    /* Stop 1xx retransmission timer, if any */
	    tsx_cancel_timer(tsx, &tsx->retransmit_timer);

Where tsx can be obtained from pjsip_inv_answer()'s inv->invite_tsx.

Which is effectively the same thing as pjsip_tsx_stop_retransmit().


Unfortunately, I don't think this will completely eliminate the deadlock because the timer callback could already be running in the monitor thread when you try to cancel the timer.


-- 
To view, visit https://gerrit.asterisk.org/9477
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic381004a3a212fe1d8eca0e707fe09dba4a6ab4e
Gerrit-Change-Number: 9477
Gerrit-PatchSet: 8
Gerrit-Owner: Torrey Searle <tsearle at gmail.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Torrey Searle <tsearle at gmail.com>
Gerrit-Comment-Date: Wed, 25 Jul 2018 15:50:16 +0000
Gerrit-HasComments: No
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180725/b8548103/attachment.html>