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 3 (Latest)

1 2 3
1 2 3

  1. trunk/channels/chan_sip.c: Loading...
trunk/channels/chan_sip.c
Revision 308944 New Change
[20] 27963 lines
[+20] [+] static int sip_set_udptl_peer(struct ast_channel *chan, struct ast_udptl *udptl)
27964

    
   
27964

   
27965
	p = chan->tech_pvt;
27965
	p = chan->tech_pvt;
27966
	if (!p) {
27966
	if (!p) {
27967
		return -1;
27967
		return -1;
27968
	}
27968
	}

    
   
27969
	/*

    
   
27970
	 * Lock both the pvt and it's owner safely.

    
   
27971
	 */
27969
	sip_pvt_lock(p);
27972
	sip_pvt_lock(p);

    
   
27973
	while (p->owner && ast_channel_trylock(p->owner)) {

    
   
27974
		sip_pvt_unlock(p);

    
   
27975
		usleep(1);

    
   
27976
		sip_pvt_lock(p);

    
   
27977
	}

    
   
27978

   

    
   
27979
	if (!p->owner) {

    
   
27980
		sip_pvt_unlock(p);

    
   
27981
		return 0;

    
   
27982
	}
27970
	if (udptl) {
27983
	if (udptl) {
27971
		ast_udptl_get_peer(udptl, &p->udptlredirip);
27984
		ast_udptl_get_peer(udptl, &p->udptlredirip);
27972
	} else {
27985
	} else {
27973
		memset(&p->udptlredirip, 0, sizeof(p->udptlredirip));
27986
		memset(&p->udptlredirip, 0, sizeof(p->udptlredirip));
27974
	}
27987
	}
[+20] [20] 8 lines
[+20] static int sip_set_udptl_peer(struct ast_channel *chan, struct ast_udptl *udptl)
27983
			ast_set_flag(&p->flags[0], SIP_NEEDREINVITE);
27996
			ast_set_flag(&p->flags[0], SIP_NEEDREINVITE);
27984
		}
27997
		}
27985
	}
27998
	}
27986
	/* Reset lastrtprx timer */
27999
	/* Reset lastrtprx timer */
27987
	p->lastrtprx = p->lastrtptx = time(NULL);
28000
	p->lastrtprx = p->lastrtptx = time(NULL);

    
   
28001
	ast_channel_unlock(p->owner);
27988
	sip_pvt_unlock(p);
28002
	sip_pvt_unlock(p);
27989
	return 0;
28003
	return 0;
27990
}
28004
}
27991

    
   
28005

   
27992
static enum ast_rtp_glue_result sip_get_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance **instance)
28006
static enum ast_rtp_glue_result sip_get_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance **instance)
[+20] [20] 107 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  */
28114
	/* Disable early RTP bridge  */
28101
	if (!ast_bridged_channel(chan) && !sip_cfg.directrtpsetup) { 	/* We are in early state */
28115
	if (!ast_bridged_channel(chan) && !sip_cfg.directrtpsetup) { 	/* We are in early state */
28102
		return 0;
28116
		return 0;
28103
	}
28117
	}
28104

    
   
28118

   
28105
	ast_channel_lock(chan);
28119
	/*

    
   
28120
	 * Lock both the pvt and it's owner safely.

    
   
28121
	 */
28106
	sip_pvt_lock(p);
28122
	sip_pvt_lock(p);

    
   
28123
	while (p->owner && ast_channel_trylock(p->owner)) {

    
   
28124
		sip_pvt_unlock(p);

    
   
28125
		usleep(1);

    
   
28126
		sip_pvt_lock(p);

    
   
28127
	}

    
   
28128

   

    
   
28129
	if (!p->owner) {

    
   
28130
		sip_pvt_unlock(p);

    
   
28131
		return 0;

    
   
28132
	}

    
   
28133

   
28107
	if (p->alreadygone) {
28134
	if (p->alreadygone) {
28108
		/* If we're destroyed, don't bother */
28135
		/* If we're destroyed, don't bother */

    
   
28136
		ast_channel_unlock(p->owner);
28109
		sip_pvt_unlock(p);
28137
		sip_pvt_unlock(p);
28110
		ast_channel_unlock(chan);

   
28111
		return 0;
28138
		return 0;
28112
	}
28139
	}
28113

    
   
28140

   
28114
	/* if this peer cannot handle reinvites of the media stream to devices
28141
	/* 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
28142
	   that are known to be behind a NAT, then stop the process now
28116
	*/
28143
	*/
28117
	if (nat_active && !ast_test_flag(&p->flags[0], SIP_DIRECT_MEDIA_NAT)) {
28144
	if (nat_active && !ast_test_flag(&p->flags[0], SIP_DIRECT_MEDIA_NAT)) {

    
   
28145
		ast_channel_unlock(p->owner);
28118
		sip_pvt_unlock(p);
28146
		sip_pvt_unlock(p);
28119
		ast_channel_unlock(chan);

   
28120
		return 0;
28147
		return 0;
28121
	}
28148
	}
28122

    
   
28149

   
28123
	if (instance) {
28150
	if (instance) {
28124
		changed |= ast_rtp_instance_get_and_cmp_remote_address(instance, &p->redirip);
28151
		changed |= ast_rtp_instance_get_and_cmp_remote_address(instance, &p->redirip);
[+20] [20] 31 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)
28156
			ast_set_flag(&p->flags[0], SIP_NEEDREINVITE);
28183
			ast_set_flag(&p->flags[0], SIP_NEEDREINVITE);
28157
		}
28184
		}
28158
	}
28185
	}
28159
	/* Reset lastrtprx timer */
28186
	/* Reset lastrtprx timer */
28160
	p->lastrtprx = p->lastrtptx = time(NULL);
28187
	p->lastrtprx = p->lastrtptx = time(NULL);

    
   
28188
	ast_channel_unlock(p->owner);
28161
	sip_pvt_unlock(p);
28189
	sip_pvt_unlock(p);
28162
	ast_channel_unlock(chan);

   
28163
	return 0;
28190
	return 0;
28164
}
28191
}
28165

    
   
28192

   
28166
static void sip_get_codec(struct ast_channel *chan, struct ast_format_cap *result)
28193
static void sip_get_codec(struct ast_channel *chan, struct ast_format_cap *result)
28167
{
28194
{
[+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.