[asterisk-commits] bebuild: branch certified-13.1 r429863 - in /certified/branches/13.1: ./ chan...

[SVN commits to the Asterisk project]

Author: bebuild
Date: Fri Dec 19 14:40:13 2014
New Revision: 429863

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=429863
Log:
chan_pjsip: Race between channel answer and bridge setup when using direct media

When direct media is enabled and a pjsip channel is answered a race would occur
between the handling of the answer and bridge setup. Sometimes the media
negotiation would take place after the native bridge was setup. This resulted
in a NULL media address, which in turn resulted in Asterisk using its address
as the remote media address when sending a reinvite.  This patch makes the
chan_pjsip answer handler synchronous thus alleviating the race condition (the
bridge won't start setting things up until after it returns).

ASTERISK-24563 #close
Reported by: Steve Pitts
Review: https://reviewboard.asterisk.org/r/4257/
........

Merged revisions 429477 from http://svn.asterisk.org/svn/asterisk/branches/13

Modified:
    certified/branches/13.1/   (props changed)
    certified/branches/13.1/channels/chan_pjsip.c

Propchange: certified/branches/13.1/
------------------------------------------------------------------------------
--- branch-13-merged (original)
+++ branch-13-merged Fri Dec 19 14:40:13 2014
@@ -1,1 +1,1 @@
-/branches/13:429196,429540,429571,429739,429829
+/branches/13:429196,429477,429540,429571,429739,429829

Modified: certified/branches/13.1/channels/chan_pjsip.c
URL: http://svnview.digium.com/svn/asterisk/certified/branches/13.1/channels/chan_pjsip.c?view=diff&rev=429863&r1=429862&r2=429863
==============================================================================
--- certified/branches/13.1/channels/chan_pjsip.c (original)
+++ certified/branches/13.1/channels/chan_pjsip.c Fri Dec 19 14:40:13 2014
@@ -490,7 +490,6 @@
 	struct ast_sip_session *session = data;
 
 	if (session->inv_session->state == PJSIP_INV_STATE_DISCONNECTED) {
-		ao2_ref(session, -1);
 		return 0;
 	}
 
@@ -507,8 +506,6 @@
 		ast_sip_session_send_response(session, packet);
 	}
 
-	ao2_ref(session, -1);
-
 	return (status == PJ_SUCCESS) ? 0 : -1;
 }
 
@@ -516,19 +513,27 @@
 static int chan_pjsip_answer(struct ast_channel *ast)
 {
 	struct ast_sip_channel_pvt *channel = ast_channel_tech_pvt(ast);
+	struct ast_sip_session *session;
 
 	if (ast_channel_state(ast) == AST_STATE_UP) {
 		return 0;
 	}
 
 	ast_setstate(ast, AST_STATE_UP);
-
-	ao2_ref(channel->session, +1);
-	if (ast_sip_push_task(channel->session->serializer, answer, channel->session)) {
+	session = ao2_bump(channel->session);
+
+	/* the answer task needs to be pushed synchronously otherwise a race condition
+	   can occur between this thread and bridging (specifically when native bridging
+	   attempts to do direct media) */
+	ast_channel_unlock(ast);
+	if (ast_sip_push_task_synchronous(session->serializer, answer, session)) {
 		ast_log(LOG_WARNING, "Unable to push answer task to the threadpool. Cannot answer call\n");
-		ao2_cleanup(channel->session);
+		ao2_ref(session, -1);
+		ast_channel_lock(ast);
 		return -1;
 	}
+	ao2_ref(session, -1);
+	ast_channel_lock(ast);
 
 	return 0;
 }