[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:

> after much attempting of getting stop retransmission timers working
 > safely it seems like something immediately re-arms it.  I have come
 > to the conclusion that the only safe fix possible is my original
 > one (where group lock is acquired while modifying the tdata).
 > 
 > Let me know what you think

Your original patch did locking but that only protected access to the tdata while in pjsip_inv_answer().  I think it likely introduced locking inversions that can cause other deadlock scenarios.  It also did not provide protection from Asterisk adding headers to the tdata since Asterisk believes that it is the only thread that knows about it.

Canceling the retransmission timer was a good idea but canceling will only work if the callback is not already in progress as that callback could restart the timer and it has to access the tdata struct.

I really think the only safe thing left is to clone the tdata before pjsip_inv_answer() passes it back to the caller.


-- 
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: Thu, 26 Jul 2018 15:59:45 +0000
Gerrit-HasComments: No
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180726/6c0624f1/attachment.html>