diff options
| author | George Joseph <gjoseph@digium.com> | 2017-09-01 04:17:02 -0600 |
|---|---|---|
| committer | Richard Mudgett <rmudgett@digium.com> | 2017-09-06 13:00:49 -0500 |
| commit | 94091c7b960aa45923036cd4d79929dcb3178cdc (patch) | |
| tree | 4c045660fa8987fe3cf28a3d1bf075d0bc23c3e7 /main | |
| parent | 2857a3334aa430b81baac31bdfe0efb3871f37d4 (diff) | |
stasis/control: Fix possible deadlock with swap channel
If an error occurs during a bridge impart it's possible that
the "bridge_after" callback might try to run before
control_swap_channel_in_bridge has been signalled to continue.
Since control_swap_channel_in_bridge is holding the control lock
and the callback needs it, a deadlock will occur.
* control_swap_channel_in_bridge now only holds the control
lock while it's actually modifying the control structure and
releases it while the bridge impart is running.
* bridge_after_cb is now tolerant of impart failures.
Change-Id: Ifd239aa93955b3eb475521f61e284fcb0da2c3b3
Diffstat (limited to 'main')
| -rw-r--r-- | main/bridge.c | 6 | ||||
| -rw-r--r-- | main/bridge_after.c | 30 |
2 files changed, 19 insertions, 17 deletions
diff --git a/main/bridge.c b/main/bridge.c index b732d5fc5..ab12ecf64 100644 --- a/main/bridge.c +++ b/main/bridge.c @@ -1758,12 +1758,13 @@ join_exit:; static void *bridge_channel_depart_thread(void *data) { struct ast_bridge_channel *bridge_channel = data; + int res = 0; if (bridge_channel->callid) { ast_callid_threadassoc_add(bridge_channel->callid); } - bridge_channel_internal_join(bridge_channel); + res = bridge_channel_internal_join(bridge_channel); /* * cleanup @@ -1775,7 +1776,8 @@ static void *bridge_channel_depart_thread(void *data) ast_bridge_features_destroy(bridge_channel->features); bridge_channel->features = NULL; - ast_bridge_discard_after_callback(bridge_channel->chan, AST_BRIDGE_AFTER_CB_REASON_DEPART); + ast_bridge_discard_after_callback(bridge_channel->chan, + res ? AST_BRIDGE_AFTER_CB_REASON_IMPART_FAILED : AST_BRIDGE_AFTER_CB_REASON_DEPART); /* If join failed there will be impart threads waiting. */ bridge_channel_impart_signal(bridge_channel->chan); ast_bridge_discard_after_goto(bridge_channel->chan); diff --git a/main/bridge_after.c b/main/bridge_after.c index d4aec75d0..69c8f989b 100644 --- a/main/bridge_after.c +++ b/main/bridge_after.c @@ -293,23 +293,23 @@ int ast_bridge_set_after_callback(struct ast_channel *chan, ast_bridge_after_cb return 0; } -const char *reason_strings[] = { - [AST_BRIDGE_AFTER_CB_REASON_DESTROY] = "Channel destroyed (hungup)", - [AST_BRIDGE_AFTER_CB_REASON_REPLACED] = "Callback was replaced", - [AST_BRIDGE_AFTER_CB_REASON_MASQUERADE] = "Channel masqueraded", - [AST_BRIDGE_AFTER_CB_REASON_DEPART] = "Channel was departed from bridge", - [AST_BRIDGE_AFTER_CB_REASON_REMOVED] = "Callback was removed", -}; - const char *ast_bridge_after_cb_reason_string(enum ast_bridge_after_cb_reason reason) { - if (reason < AST_BRIDGE_AFTER_CB_REASON_DESTROY - || AST_BRIDGE_AFTER_CB_REASON_REMOVED < reason - || !reason_strings[reason]) { - return "Unknown"; - } - - return reason_strings[reason]; + switch (reason) { + case AST_BRIDGE_AFTER_CB_REASON_DESTROY: + return "Channel destroyed (hungup)"; + case AST_BRIDGE_AFTER_CB_REASON_REPLACED: + return "Callback was replaced"; + case AST_BRIDGE_AFTER_CB_REASON_MASQUERADE: + return "Channel masqueraded"; + case AST_BRIDGE_AFTER_CB_REASON_DEPART: + return "Channel was departed from bridge"; + case AST_BRIDGE_AFTER_CB_REASON_REMOVED: + return "Callback was removed"; + case AST_BRIDGE_AFTER_CB_REASON_IMPART_FAILED: + return "Channel failed joining the bridge"; + } + return "Unknown"; } struct after_bridge_goto_ds { |