Review Board 1.7.16


fix Deadlock with attended transfers of SIP calls

Review Request #1126 - Created Feb. 23, 2011 and submitted

Alec Davis
trunk
18468, 18491, 18734, 18837
Reviewers
asterisk-dev
Asterisk
in 1.8.2.3 and prior to trunk r306216, when the REFER transfer is complete then a deadlock occurs.

Reason being sip_set_rtp_peer locks pvt, then pbx_builtin_getvar_helper tries to lock chan
Violating locking order.
 
=== Thread ID: -1292625008 (do_monitor started at [24470] chan_sip.c restart_monitor())
=== ---> Lock #0 (chan_sip.c): MUTEX 23964 handle_request_do &netlock 0xb6796e80 (1)
=== ---> Lock #1 (channel.c): MUTEX 6211 ast_do_masquerade channels 0x8d4e0c8 (1)
=== ---> Lock #2 (channel.c): MUTEX 6214 ast_do_masquerade original 0xbd98f48 (1)
=== ---> Lock #3 (channel.c): MUTEX 6234 ast_do_masquerade clonechan 0xb24bf7d0 (1)
=== ---> Waiting for Lock #4 (chan_sip.c): MUTEX 6164 sip_fixup p 0xb24bab10 (1)
=== --- ---> Locked Here: chan_sip.c line 27632 (sip_set_rtp_peer)

=== -------------------------------------------------------------------
===
=== Thread ID: -1315861616 (pbx_thread started at [ 5035] pbx.c ast_pbx_start())
=== ---> Lock #0 (chan_sip.c): MUTEX 27632 sip_set_rtp_peer p 0xb24bab10 (1)
=== ---> Waiting for Lock #1 (pbx.c): MUTEX 9467 pbx_builtin_getvar_helper chan 0xb24bf7d0 (1)
=== --- ---> Locked Here: channel.c line 6234 (ast_do_masquerade)

multiple other bug reports reveal the sip_set_rtp_peer pvt locking issue.

Solution:
  inc pvt refcount to prevent the possibility of it disappearing while unlocked.
  unlock the pvt
    call transmit_reinvite_with_sdp (which finishes up invoking pbx_builtin_getvar_helper)
  lock the pvt
  dec pvt refcount

Applies to 1.8.2.3 and trunk. Both verified deadlocks.
Presumably 1.8 branch

r306216 tries to solve it, but causes more channel locking issues 
see https://issues.asterisk.org/view.php?id=18837#132337
Testing: trunk

3 SIP phones.

A calls B, and B answers on line 1.
B puts A on hold by selecting line2.
B calls C, and C answers.
B initiates transfer of line1 to line2, phone uses REFER.

Diff revision 1

This is not the most recent revision of the diff. The latest diff is revision 3. See what's changed.

1 2 3
1 2 3

  1. trunk/channels/chan_sip.c: Loading...
trunk/channels/chan_sip.c
Revision 308720 New Change
[20] 28099 lines
[+20] [+] static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *instance, struct ast_rtp_instance *vinstance, struct ast_rtp_instance *tinstance, const struct ast_format_cap *cap, int nat_active)
28100
	/* Disable early RTP bridge  */
28100
	/* Disable early RTP bridge  */
28101
	if (!ast_bridged_channel(chan) && !sip_cfg.directrtpsetup) { 	/* We are in early state */
28101
	if (!ast_bridged_channel(chan) && !sip_cfg.directrtpsetup) { 	/* We are in early state */
28102
		return 0;
28102
		return 0;
28103
	}
28103
	}
28104

    
   
28104

   
28105
	ast_channel_lock(chan);

   
28106
	sip_pvt_lock(p);
28105
	sip_pvt_lock(p);
28107
	if (p->alreadygone) {
28106
	if (p->alreadygone) {
28108
		/* If we're destroyed, don't bother */
28107
		/* If we're destroyed, don't bother */
28109
		sip_pvt_unlock(p);
28108
		sip_pvt_unlock(p);
28110
		ast_channel_unlock(chan);

   
28111
		return 0;
28109
		return 0;
28112
	}
28110
	}
28113

    
   
28111

   
28114
	/* if this peer cannot handle reinvites of the media stream to devices
28112
	/* if this peer cannot handle reinvites of the media stream to devices
28115
	   that are known to be behind a NAT, then stop the process now
28113
	   that are known to be behind a NAT, then stop the process now
28116
	*/
28114
	*/
28117
	if (nat_active && !ast_test_flag(&p->flags[0], SIP_DIRECT_MEDIA_NAT)) {
28115
	if (nat_active && !ast_test_flag(&p->flags[0], SIP_DIRECT_MEDIA_NAT)) {
28118
		sip_pvt_unlock(p);
28116
		sip_pvt_unlock(p);
28119
		ast_channel_unlock(chan);

   
28120
		return 0;
28117
		return 0;
28121
	}
28118
	}
28122

    
   
28119

   

    
   
28120
	dialog_ref(p, "in sip_set_rtp_peer: Inc refs so pvt doesn't accidentally become dead before we are done");

    
   
28121

   
28123
	if (instance) {
28122
	if (instance) {
28124
		changed |= ast_rtp_instance_get_and_cmp_remote_address(instance, &p->redirip);
28123
		changed |= ast_rtp_instance_get_and_cmp_remote_address(instance, &p->redirip);
28125
	} else if (!ast_sockaddr_isnull(&p->redirip)) {
28124
	} else if (!ast_sockaddr_isnull(&p->redirip)) {
28126
		memset(&p->redirip, 0, sizeof(p->redirip));
28125
		memset(&p->redirip, 0, sizeof(p->redirip));
28127
		changed = 1;
28126
		changed = 1;
[+20] [20] 19 lines
[+20] static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *instance, struct ast_rtp_instance *vinstance, struct ast_rtp_instance *tinstance, const struct ast_format_cap *cap, int nat_active)
28147
			if (p->do_history)
28146
			if (p->do_history)
28148
				append_history(p, "ExtInv", "Initial invite sent with remote bridge proposal.");
28147
				append_history(p, "ExtInv", "Initial invite sent with remote bridge proposal.");
28149
			ast_debug(1, "Early remote bridge setting SIP '%s' - Sending media to %s\n", p->callid, ast_sockaddr_stringify(instance ? &p->redirip : &p->ourip));
28148
			ast_debug(1, "Early remote bridge setting SIP '%s' - Sending media to %s\n", p->callid, ast_sockaddr_stringify(instance ? &p->redirip : &p->ourip));
28150
		} else if (!p->pendinginvite) {	 /* We are up, and have no outstanding invite */
28149
		} else if (!p->pendinginvite) {	 /* We are up, and have no outstanding invite */
28151
			ast_debug(3, "Sending reinvite on SIP '%s' - It's audio soon redirected to IP %s\n", p->callid, ast_sockaddr_stringify(instance ? &p->redirip : &p->ourip));
28150
			ast_debug(3, "Sending reinvite on SIP '%s' - It's audio soon redirected to IP %s\n", p->callid, ast_sockaddr_stringify(instance ? &p->redirip : &p->ourip));

    
   
28151
			sip_pvt_unlock(p);
28152
			transmit_reinvite_with_sdp(p, FALSE, FALSE);
28152
			transmit_reinvite_with_sdp(p, FALSE, FALSE);

    
   
28153
			sip_pvt_lock(p);
28153
		} else if (!ast_test_flag(&p->flags[0], SIP_PENDINGBYE)) {
28154
		} else if (!ast_test_flag(&p->flags[0], SIP_PENDINGBYE)) {
28154
			ast_debug(3, "Deferring reinvite on SIP '%s' - It's audio will be redirected to IP %s\n", p->callid, ast_sockaddr_stringify(instance ? &p->redirip : &p->ourip));
28155
			ast_debug(3, "Deferring reinvite on SIP '%s' - It's audio will be redirected to IP %s\n", p->callid, ast_sockaddr_stringify(instance ? &p->redirip : &p->ourip));
28155
			/* We have a pending Invite. Send re-invite when we're done with the invite */
28156
			/* We have a pending Invite. Send re-invite when we're done with the invite */
28156
			ast_set_flag(&p->flags[0], SIP_NEEDREINVITE);
28157
			ast_set_flag(&p->flags[0], SIP_NEEDREINVITE);
28157
		}
28158
		}
28158
	}
28159
	}
28159
	/* Reset lastrtprx timer */
28160
	/* Reset lastrtprx timer */
28160
	p->lastrtprx = p->lastrtptx = time(NULL);
28161
	p->lastrtprx = p->lastrtptx = time(NULL);
28161
	sip_pvt_unlock(p);
28162
	sip_pvt_unlock(p);
28162
	ast_channel_unlock(chan);
28163
	dialog_unref(p, "in sip_set_rtp_peer: Dec ref count so pvt can become dead");
28163
	return 0;
28164
	return 0;
28164
}
28165
}
28165

    
   
28166

   
28166
static void sip_get_codec(struct ast_channel *chan, struct ast_format_cap *result)
28167
static void sip_get_codec(struct ast_channel *chan, struct ast_format_cap *result)
28167
{
28168
{
[+20] [20] 1287 lines
  1. trunk/channels/chan_sip.c: Loading...

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.