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

(2 comments)

https://gerrit.asterisk.org/#/c/3111/1//COMMIT_MSG
Commit Message:

Line 9: It is possible in certain situations for a session refresh to be invoked
> Is there an example scenario that you can describe?
It's honestly a pretty weird scenario that occurred while I was playing around with a completely different patch. We ended up sending a re-INVITE - and actually *wanted* to send said re-INVITE - on a channel that had only a partially set up session on it. That is, we have called ast_request to get the channel from chan_pjsip - which in turn creates the session via ast_sip_session_create_outgoing. However, by the time we called ast_sip_session_refresh, we had not yet sent the initial INVITE request.

Without this patch, that will cause a crash. However, that crash isn't because PJSIP can't send the INVITE request - it's simply because we're calling functions that inspect an inv_session without first checking if it exists. If we look to see if the inv_session exists first, we'll end up sending the INVITE that we asked for in ast_sip_session_refresh like we ostensibly wanted.

At this point, said crash is definitely theoretical, and shouldn't be possible with the existing code. But since ast_sip_session_refresh is a public function, we should do "the right thing" and be somewhat tolerant of the state of the ast_sip_session that is passed to us.


https://gerrit.asterisk.org/#/c/3111/1/res/res_pjsip_session.c
File res/res_pjsip_session.c:

Line 921: 		if (inv_session->neg && pjmedia_sdp_neg_get_state(inv_session->neg) != PJMEDIA_SDP_NEG_STATE_DONE) {
> I take it since we don't have a negotiator that one will be created soon?  
The negotiator will get created when we call pjsip_inv_reinvite. It will create a negotiator if one does not exist:

2873     if (new_offer) {
2874         if (!inv->neg) {
2875             status = pjmedia_sdp_neg_create_w_local_offer(inv->pool, 
2876                                                           new_offer,
2877                                                           &inv->neg);
2878             if (status != PJ_SUCCESS)
2879                 goto on_return;
2880


-- 
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: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes