kenmirrors/asterisk
1
0
Fork 0
mirror of https://github.com/asterisk/asterisk.git synced 2025-09-29 18:19:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

chan_pjsip: Fix deadlock when masquerading PJSIP channels.

Browse Source 
Performing a directed call pickup resulted in a deadlock when PJSIP
channels were involved.

A masquerade needs to hold onto the channel locks while it swaps channel
information between the two channels involved in the masquerade.  With
PJSIP channels, the fixup routine needed to push a fixup task onto the
PJSIP channel's serializer.  Unfortunately, if the serializer was also
processing a task that needed to lock the channel, you get deadlock.

* Added a new control frame that is used to notify the channels that a
masquerade is about to start and when it has completed.

* Added the ability to query taskprocessors if the current thread is the
taskprocessor thread.

* Added the ability to suspend/unsuspend the PJSIP serializer thread so a
masquerade could fixup the PJSIP channel without using the serializer.

ASTERISK-24356 #close
Reported by: rmudgett

Review: https://reviewboard.asterisk.org/r/4034/
........

Merged revisions 424471 from http://svn.asterisk.org/svn/asterisk/branches/12
........

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


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@424473 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
Richard Mudgett
2014-10-03 17:47:42 +00:00
parent 4967478d18
commit 0165c5f95a
18 changed files with 253 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
main/bridge_channel.c
View File

@@ -2065,6 +2065,10 @@ static void bridge_channel_handle_control(struct ast_bridge_channel *bridge_chan
ast_indicate(chan, -1);
}
break;
case AST_CONTROL_MASQUERADE_NOTIFY:
/* Should never happen. */
ast_assert(0);
break;
default:
ast_indicate_data(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
break;

29
main/channel.c
View File

@@ -4285,6 +4285,7 @@ static int attribute_const is_visible_indication(enum ast_control_frame_type con
case AST_CONTROL_MCID:
case AST_CONTROL_UPDATE_RTP_PEER:
case AST_CONTROL_PVT_CAUSE_CODE:
case AST_CONTROL_MASQUERADE_NOTIFY:
case AST_CONTROL_STREAM_STOP:
case AST_CONTROL_STREAM_SUSPEND:
case AST_CONTROL_STREAM_REVERSE:
@@ -4451,7 +4452,9 @@ int ast_indicate_data(struct ast_channel *chan, int _condition,
ast_channel_lock(chan);
/* Don't bother if the channel is about to go away, anyway. */
if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_ZOMBIE) || ast_check_hangup(chan)) {
if ((ast_test_flag(ast_channel_flags(chan), AST_FLAG_ZOMBIE)
|| ast_check_hangup(chan))
&& condition != AST_CONTROL_MASQUERADE_NOTIFY) {
res = -1;
goto indicate_cleanup;
}
@@ -4599,6 +4602,7 @@ int ast_indicate_data(struct ast_channel *chan, int _condition,
case AST_CONTROL_AOC:
case AST_CONTROL_END_OF_Q:
case AST_CONTROL_MCID:
case AST_CONTROL_MASQUERADE_NOTIFY:
case AST_CONTROL_UPDATE_RTP_PEER:
case AST_CONTROL_STREAM_STOP:
case AST_CONTROL_STREAM_SUSPEND:
@@ -6445,6 +6449,11 @@ static void channel_do_masquerade(struct ast_channel *original, struct ast_chann
* original channel's backend. While the features are nice, which is the
* reason we're keeping it, it's still awesomely weird. XXX */
/* Indicate to each channel that a masquerade is about to begin. */
x = 1;
ast_indicate_data(original, AST_CONTROL_MASQUERADE_NOTIFY, &x, sizeof(x));
ast_indicate_data(clonechan, AST_CONTROL_MASQUERADE_NOTIFY, &x, sizeof(x));
/*
* The container lock is necessary for proper locking order
* because the channels must be unlinked to change their
@@ -6485,8 +6494,9 @@ static void channel_do_masquerade(struct ast_channel *original, struct ast_chann
/* Start the masquerade channel contents rearangement. */
ast_channel_lock_both(original, clonechan);
ast_debug(4, "Actually Masquerading %s(%u) into the structure of %s(%u)\n",
ast_channel_name(clonechan), ast_channel_state(clonechan), ast_channel_name(original), ast_channel_state(original));
ast_debug(1, "Actually Masquerading %s(%u) into the structure of %s(%u)\n",
ast_channel_name(clonechan), ast_channel_state(clonechan),
ast_channel_name(original), ast_channel_state(original));
/*
* Remember the original read/write formats. We turn off any
@@ -6759,6 +6769,19 @@ static void channel_do_masquerade(struct ast_channel *original, struct ast_chann
ast_channel_unlock(original);
ast_channel_unlock(clonechan);
/*
* Indicate to each channel that a masquerade is complete.
*
* We can still do this to clonechan even though it is a
* zombie because ast_indicate_data() will explicitly pass
* this control and ast_hangup() is held off until the
* ast_channel_masq() and ast_channel_masqr() pointers are
* cleared.
*/
x = 0;
ast_indicate_data(original, AST_CONTROL_MASQUERADE_NOTIFY, &x, sizeof(x));
ast_indicate_data(clonechan, AST_CONTROL_MASQUERADE_NOTIFY, &x, sizeof(x));
ast_bridge_notify_masquerade(original);
if (clone_hold_state == AST_CONTROL_HOLD) {

6
main/core_unreal.c
View File

@@ -530,6 +530,12 @@ int ast_unreal_indicate(struct ast_channel *ast, int condition, const void *data
ao2_ref(p, 1); /* ref for unreal_queue_frame */
switch (condition) {
case AST_CONTROL_MASQUERADE_NOTIFY:
/*
* Always block this because this is the channel being
* masqueraded; not anything down the chain.
*/
break;
case AST_CONTROL_CONNECTED_LINE:
case AST_CONTROL_REDIRECTING:
res = unreal_colp_redirect_indicate(p, ast, condition);

18
main/taskprocessor.c
View File

@@ -78,9 +78,9 @@ struct ast_taskprocessor {
long tps_queue_size;
/*! \brief Taskprocessor queue */
AST_LIST_HEAD_NOLOCK(tps_queue, tps_task) tps_queue;
/*! \brief Taskprocessor singleton list entry */
AST_LIST_ENTRY(ast_taskprocessor) list;
struct ast_taskprocessor_listener *listener;
/*! Current thread executing the tasks */
pthread_t thread;
/*! Indicates if the taskprocessor is currently executing a task */
unsigned int executing:1;
};
@@ -600,6 +600,8 @@ static struct ast_taskprocessor *__allocate_taskprocessor(const char *name, stru
ao2_ref(listener, +1);
p->listener = listener;
p->thread = AST_PTHREADT_NULL;
ao2_ref(p, +1);
listener->tps = p;
@@ -752,6 +754,7 @@ int ast_taskprocessor_execute(struct ast_taskprocessor *tps)
return 0;
}
tps->thread = pthread_self();
tps->executing = 1;
if (t->wants_local) {
@@ -768,6 +771,7 @@ int ast_taskprocessor_execute(struct ast_taskprocessor *tps)
tps_task_free(t);
ao2_lock(tps);
tps->thread = AST_PTHREADT_NULL;
/* We need to check size in the same critical section where we reset the
* executing bit. Avoids a race condition where a task is pushed right
* after we pop an empty stack.
@@ -789,3 +793,13 @@ int ast_taskprocessor_execute(struct ast_taskprocessor *tps)
}
return size > 0;
}
int ast_taskprocessor_is_task(struct ast_taskprocessor *tps)
{
int is_task;
ao2_lock(tps);
is_task = pthread_equal(tps->thread, pthread_self());
ao2_unlock(tps);
return is_task;
}