[svn-commits] mmichelson: branch 13 r422542 - in /branches/13: ./ channels/ include/asteris...
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Tue Sep 2 15:29:18 CDT 2014
Author: mmichelson
Date: Tue Sep 2 15:29:09 2014
New Revision: 422542
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=422542
Log:
Resolve race condition where channels enter dialplan application before media has been negotiated.
Testsuite tests will occasionally fail because on reception of a 200 OK SIP response,
an AST_CONTROL_ANSWER frame is queued prior to when media has finished being
negotiated. This is because session supplements are called into before PJSIP's
inv_session code has told us that media has been updated. Sometimes the queued answer
frame is handled by the PBX thread before the ensuing media negotiations occur, causing
a test failure.
As it turns out, there is another place that session supplements could be called into, which is
after media has finished getting negotiated. What this commit introduces is a means for session
supplements to indicate when they wish to be called into when handling an incoming SIP response.
By default, all session supplements will be run at the same point that they were prior to this
commit. However, session supplements may indicate that they wish to be handled earlier than
normal on redirects, or they may indicate they wish to be handled after media has been negotiated.
In this changeset, two session supplements have been updated to indicate a preference for when
they should be run: res_pjsip_diversion executes before handling redirection in order to get
information from the Diversion header, and chan_pjsip now handles responses to INVITEs after
media negotiation to fix the race condition mentioned previously.
ASTERISK-24212 #close
Reported by Matt Jordan
Review: https://reviewboard.asterisk.org/r/3930
........
Merged revisions 422536 from http://svn.asterisk.org/svn/asterisk/branches/12
Modified:
branches/13/ (props changed)
branches/13/channels/chan_pjsip.c
branches/13/include/asterisk/res_pjsip_session.h
branches/13/res/res_pjsip_diversion.c
branches/13/res/res_pjsip_session.c
Propchange: branches/13/
------------------------------------------------------------------------------
Binary property 'branch-12-merged' - no diff available.
Modified: branches/13/channels/chan_pjsip.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/channels/chan_pjsip.c?view=diff&rev=422542&r1=422541&r2=422542
==============================================================================
--- branches/13/channels/chan_pjsip.c (original)
+++ branches/13/channels/chan_pjsip.c Tue Sep 2 15:29:09 2014
@@ -145,6 +145,8 @@
.session_end = chan_pjsip_session_end,
.incoming_request = chan_pjsip_incoming_request,
.incoming_response = chan_pjsip_incoming_response,
+ /* It is important that this supplement runs after media has been negotiated */
+ .response_priority = AST_SIP_SESSION_AFTER_MEDIA,
};
static int chan_pjsip_incoming_ack(struct ast_sip_session *session, struct pjsip_rx_data *rdata);
@@ -2029,6 +2031,11 @@
struct ast_features_pickup_config *pickup_cfg = ast_get_chan_features_pickup_config(session->channel);
struct ast_channel *chan;
+ /* We don't care about reinvites */
+ if (session->inv_session->state >= PJSIP_INV_STATE_CONFIRMED) {
+ return 0;
+ }
+
if (!pickup_cfg) {
ast_log(LOG_ERROR, "Unable to retrieve pickup configuration options. Unable to detect call pickup extension.\n");
return 0;
@@ -2070,6 +2077,11 @@
static int pbx_start_incoming_request(struct ast_sip_session *session, pjsip_rx_data *rdata)
{
int res;
+
+ /* We don't care about reinvites */
+ if (session->inv_session->state >= PJSIP_INV_STATE_CONFIRMED) {
+ return 0;
+ }
res = ast_pbx_start(session->channel);
Modified: branches/13/include/asterisk/res_pjsip_session.h
URL: http://svnview.digium.com/svn/asterisk/branches/13/include/asterisk/res_pjsip_session.h?view=diff&rev=422542&r1=422541&r2=422542
==============================================================================
--- branches/13/include/asterisk/res_pjsip_session.h (original)
+++ branches/13/include/asterisk/res_pjsip_session.h Tue Sep 2 15:29:09 2014
@@ -141,6 +141,36 @@
typedef int (*ast_sip_session_sdp_creation_cb)(struct ast_sip_session *session, pjmedia_sdp_session *sdp);
/*!
+ * \brief Describes when a supplement should be called into on incoming responses.
+ *
+ * In most cases, session supplements will not need to worry about this because in most cases,
+ * the correct value will be automatically applied. However, there are rare circumstances
+ * when a supplement will want to specify when it should be called.
+ *
+ * The values below are listed in chronological order.
+ */
+enum ast_sip_session_response_priority {
+ /*!
+ * When processing 3XX responses, the supplement is called into before
+ * the redirecting information is processed.
+ */
+ AST_SIP_SESSION_BEFORE_REDIRECTING = (1 << 0),
+ /*!
+ * For responses to INVITE transactions, the supplement is called into
+ * before media is negotiated.
+ *
+ * This priority is applied by default to any session supplement that
+ * does not specify a response priority.
+ */
+ AST_SIP_SESSION_BEFORE_MEDIA = (1 << 1),
+ /*!
+ * For INVITE transactions, the supplement is called into after media
+ * is negotiated.
+ */
+ AST_SIP_SESSION_AFTER_MEDIA = (1 << 2),
+};
+
+/*!
* \brief A supplement to SIP message processing
*
* These can be registered by any module in order to add
@@ -214,6 +244,11 @@
void (*outgoing_response)(struct ast_sip_session *session, struct pjsip_tx_data *tdata);
/*! Next item in the list */
AST_LIST_ENTRY(ast_sip_session_supplement) next;
+ /*!
+ * Determines when the supplement is processed when handling a response.
+ * Defaults to AST_SIP_SESSION_BEFORE_MEDIA
+ */
+ enum ast_sip_session_response_priority response_priority;
};
enum ast_sip_session_sdp_stream_defer {
Modified: branches/13/res/res_pjsip_diversion.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip_diversion.c?view=diff&rev=422542&r1=422541&r2=422542
==============================================================================
--- branches/13/res/res_pjsip_diversion.c (original)
+++ branches/13/res/res_pjsip_diversion.c Tue Sep 2 15:29:09 2014
@@ -325,6 +325,7 @@
.incoming_response = diversion_incoming_response,
.outgoing_request = diversion_outgoing_request,
.outgoing_response = diversion_outgoing_response,
+ .response_priority = AST_SIP_SESSION_BEFORE_REDIRECTING,
};
static int load_module(void)
Modified: branches/13/res/res_pjsip_session.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip_session.c?view=diff&rev=422542&r1=422541&r2=422542
==============================================================================
--- branches/13/res/res_pjsip_session.c (original)
+++ branches/13/res/res_pjsip_session.c Tue Sep 2 15:29:09 2014
@@ -53,8 +53,10 @@
/* Some forward declarations */
static void handle_incoming_request(struct ast_sip_session *session, pjsip_rx_data *rdata, pjsip_event_id_e type);
-static void handle_incoming_response(struct ast_sip_session *session, pjsip_rx_data *rdata, pjsip_event_id_e type);
-static int handle_incoming(struct ast_sip_session *session, pjsip_rx_data *rdata, pjsip_event_id_e type);
+static void handle_incoming_response(struct ast_sip_session *session, pjsip_rx_data *rdata, pjsip_event_id_e type,
+ enum ast_sip_session_response_priority response_priority);
+static int handle_incoming(struct ast_sip_session *session, pjsip_rx_data *rdata, pjsip_event_id_e type,
+ enum ast_sip_session_response_priority response_priority);
static void handle_outgoing_request(struct ast_sip_session *session, pjsip_tx_data *tdata);
static void handle_outgoing_response(struct ast_sip_session *session, pjsip_tx_data *tdata);
static void handle_outgoing(struct ast_sip_session *session, pjsip_tx_data *tdata);
@@ -483,6 +485,10 @@
int inserted = 0;
SCOPED_LOCK(lock, &session_supplements, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
+ if (!supplement->response_priority) {
+ supplement->response_priority = AST_SIP_SESSION_BEFORE_MEDIA;
+ }
+
AST_RWLIST_TRAVERSE_SAFE_BEGIN(&session_supplements, iter, next) {
if (iter->priority > supplement->priority) {
AST_RWLIST_INSERT_BEFORE_CURRENT(supplement, next);
@@ -1795,27 +1801,27 @@
}
}
-static void handle_incoming_response(struct ast_sip_session *session, pjsip_rx_data *rdata, pjsip_event_id_e type)
+static void handle_incoming_response(struct ast_sip_session *session, pjsip_rx_data *rdata, pjsip_event_id_e type,
+ enum ast_sip_session_response_priority response_priority)
{
struct ast_sip_session_supplement *supplement;
struct pjsip_status_line status = rdata->msg_info.msg->line.status;
- /* Squash all redirect transaction related responses as the supplements have already been invoked */
- if (type == PJSIP_EVENT_TSX_STATE && PJSIP_IS_STATUS_IN_CLASS(status.code, 300)) {
- return;
- }
-
ast_debug(3, "Response is %d %.*s\n", status.code, (int) pj_strlen(&status.reason),
pj_strbuf(&status.reason));
AST_LIST_TRAVERSE(&session->supplements, supplement, next) {
+ if (!(supplement->response_priority & response_priority)) {
+ continue;
+ }
if (supplement->incoming_response && does_method_match(&rdata->msg_info.cseq->method.name, supplement->method)) {
supplement->incoming_response(session, rdata);
}
}
}
-static int handle_incoming(struct ast_sip_session *session, pjsip_rx_data *rdata, pjsip_event_id_e type)
+static int handle_incoming(struct ast_sip_session *session, pjsip_rx_data *rdata, pjsip_event_id_e type,
+ enum ast_sip_session_response_priority response_priority)
{
ast_debug(3, "Received %s\n", rdata->msg_info.msg->type == PJSIP_REQUEST_MSG ?
"request" : "response");
@@ -1823,7 +1829,7 @@
if (rdata->msg_info.msg->type == PJSIP_REQUEST_MSG) {
handle_incoming_request(session, rdata, type);
} else {
- handle_incoming_response(session, rdata, type);
+ handle_incoming_response(session, rdata, type, response_priority);
}
return 0;
@@ -1913,7 +1919,8 @@
handle_outgoing(session, e->body.tx_msg.tdata);
break;
case PJSIP_EVENT_RX_MSG:
- handle_incoming(session, e->body.rx_msg.rdata, type);
+ handle_incoming(session, e->body.rx_msg.rdata, type,
+ AST_SIP_SESSION_BEFORE_MEDIA);
break;
case PJSIP_EVENT_TSX_STATE:
ast_debug(3, "Source of transaction state change is %s\n", pjsip_event_str(e->body.tsx_state.type));
@@ -1923,7 +1930,8 @@
handle_outgoing(session, e->body.tsx_state.src.tdata);
break;
case PJSIP_EVENT_RX_MSG:
- handle_incoming(session, e->body.tsx_state.src.rdata, type);
+ handle_incoming(session, e->body.tsx_state.src.rdata, type,
+ AST_SIP_SESSION_BEFORE_MEDIA);
break;
case PJSIP_EVENT_TRANSPORT_ERROR:
case PJSIP_EVENT_TIMER:
@@ -1965,6 +1973,7 @@
}
switch (e->body.tsx_state.type) {
case PJSIP_EVENT_TX_MSG:
+ handle_outgoing(session, e->body.tsx_state.src.tdata);
/* When we create an outgoing request, we do not have access to the transaction that
* is created. Instead, We have to place transaction-specific data in the tdata. Here,
* we transfer the data into the transaction. This way, when we receive a response, we
@@ -1973,6 +1982,8 @@
tsx->mod_data[session_module.id] = e->body.tsx_state.src.tdata->mod_data[session_module.id];
break;
case PJSIP_EVENT_RX_MSG:
+ handle_incoming(session, e->body.tsx_state.src.rdata, e->type,
+ AST_SIP_SESSION_AFTER_MEDIA);
if (tsx->method.id == PJSIP_INVITE_METHOD) {
if (tsx->role == PJSIP_ROLE_UAC) {
if (tsx->state == PJSIP_TSX_STATE_COMPLETED) {
@@ -2004,10 +2015,6 @@
}
}
}
- } else {
- if (tsx->role == PJSIP_ROLE_UAS && tsx->state == PJSIP_TSX_STATE_TRYING) {
- handle_incoming_request(session, e->body.tsx_state.src.rdata, PJSIP_EVENT_TSX_STATE);
- }
}
if ((cb = ast_sip_mod_data_get(tsx->mod_data, session_module.id,
MOD_DATA_ON_RESPONSE))) {
@@ -2182,7 +2189,8 @@
return PJSIP_REDIRECT_STOP;
}
- handle_incoming(session, e->body.rx_msg.rdata, PJSIP_EVENT_RX_MSG);
+ handle_incoming(session, e->body.rx_msg.rdata, PJSIP_EVENT_RX_MSG,
+ AST_SIP_SESSION_BEFORE_REDIRECTING);
uri = pjsip_uri_get_uri(target);
More information about the svn-commits
mailing list