> On Dec. 4, 2014, 9:45 p.m., Matt Jordan wrote: > > /branches/12/res/res_pjsip_session.c, line 497 > > <https://reviewboard.asterisk.org/r/4233/diff/1/?file=69523#file69523line497> > > > > Completely ancillary to this... but I don't see where this gets used > > anywhere. :-)
It's used by T.38 code. It calls ast_sip_session_refresh() with a response callback. If the refresh can't be sent immediately due to a pending INVITE request, then the refresh is delayed, and the response callback is stored so that the callback can be called when a response to the session refresh is received. - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4233/#review13891 ----------------------------------------------------------- On Dec. 4, 2014, 9:22 p.m., Mark Michelson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4233/ > ----------------------------------------------------------- > > (Updated Dec. 4, 2014, 9:22 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-24556 > https://issues.asterisk.org/jira/browse/ASTERISK-24556 > > > Repository: Asterisk > > > Description > ------- > > A reporter discovered that Asterisk would crash when attempting to retransmit > a reinvite that had previously received a 491 response. The crash occurred > because a pjsip_tx_data structure was being saved for reuse, but its > reference count was not being increased. The result was that the > pjsip_tx_data was being freed before we were actually done with it. When we > attempted to re-use the structure when re-sending the reinvite, Asterisk > would crash. > > The fix implemented here is not to try holding onto the pjsip_tx_data at all. > Instead, when we reschedule sending the reinvite, we create a brand new > pjsip_tx_data and send that instead. Because of this change, there is no need > for an ast_sip_session_delayed_request structure to have a pjsip_tx_data on > it any more. So any code referencing its use has been removed. > > When this initial fix was introduced, I encountered a second crash when > processing a subsequent 200 OK on a rescheduled reinvite. The reason was that > when rescheduling the reinvite, we gave the wrong location for a response > callback. This has been fixed in this patch as well. > > (Feel free to stop reading at this point. The rest of this is justification > for the way this is being fixed) > > My initial inclination when fixing this was to fix it by bumping the > reference count of the pjsip_tx_data structure, and if necessary, > decrementing its refcount when we are finished with it. Unfortunately, PJSIP > does not allow for this. This is because when we initially send our reinvite, > PJSIP creates a transaction and registers the transaction using a key based > on the branch parameter. When we receive the 491 response, PJSIP sends an ACK > but then keeps the transaction alive for a while longer. This is so that if > our outbound ACK is lost, and we receive the 491 again, we can retransmit the > ACK. Since the transaction is still present, when we attempt to retransmit > the reinvite, PJSIP complains that a transaction with the branch we've given > it already exists. Thinking about it a bit further, I realized that in > addition to the re-used branch, we also were not increasing the CSeq value as > we should when sending out the reinvite again. Thus, holding onto the > pjsip_tx_data was a bad sol ution. Just to be certain about the rules about this, I consulted RFC 3261 to see what it says about re sending the reinvite. It says, "... the UAC SHOULD attempt the re-INVITE once more, if it still desires for that session modification to take place." The wording here is "attempt the re-INVITE once more", which does not mean to retransmit the same reinvite. So sending a new reinvite is perfectly fine. > > > Diffs > ----- > > /branches/12/res/res_pjsip_session.c 428420 > > Diff: https://reviewboard.asterisk.org/r/4233/diff/ > > > Testing > ------- > > See /r/4234 > > > Thanks, > > Mark Michelson > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
