[Asterisk-code-review] res/res pjsip session: Check for presence of an active negot... (asterisk[13])

[Matt Jordan]

Matt Jordan has posted comments on this change.

Change subject: res/res_pjsip_session: Check for presence of an active negotiator
......................................................................


Patch Set 5:

> My point is if inv_session->neg is NULL then we are trying to refresh the session so early that the initial INVITE or response to the INVITE hasn't actually been built yet much less sent.  In that case we could just return to let the other thread do its job and the refresh will be part of the initial INVITE negotiation or we could avoid any race potential and delay the refresh request.

This assumes that there is a thread that is attempting to send the initial INVITE, which is not the case in the scenario I was working on.

> Furthermore, pjmedia_sdp_neg_get_state() is NULL tolerant because of an immediate PJPROJECT PJ_ASSERT_RETURN() check.  If asserts are not enabled it returns the negotiation state as not done.  If asserts are enabled it aborts Asterisk.

The point of this patch is to not trigger that assert - as that was (one of) the crashes that I was hitting. If inv_session->neg is NULL, then I don't want to call pjmedia_sdp_neg_get_state, which is why I'm checking for that value before calling it. Switching this to an || would cause us to call pjmedia_sdp_neg_get_state with inv_session->neg, which is what I'm trying to prevent.

-- 
To view, visit https://gerrit.asterisk.org/3111
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1028323e7e01b0a531865e5412a71b6f6ec4276d
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: No