From ac843c3600f89833f75b48629600d54ce809a070 Mon Sep 17 00:00:00 2001 From: George Joseph Date: Wed, 22 Jan 2025 13:52:33 -0700 Subject: [PATCH] bridging: Fix multiple bridging issues causing SEGVs and FRACKs. Issues: * The bridging core allowed multiple bridges to be created with the same unique bridgeId at the same time. Only the last bridge created with the duplicate name was actually saved to the core bridges container. * The bridging core was creating a stasis topic for the bridge and saving it in the bridge->topic field but not increasing its reference count. In the case where two bridges were created with the same uniqueid (which is also the topic name), the second bridge would get the _existing_ topic the first bridge created. When the first bridge was destroyed, it would take the topic with it so when the second bridge attempted to publish a message to it it either FRACKed or SEGVd. * The bridge destructor, which also destroys the bridge topic, is run from the bridge manager thread not the caller's thread. This makes it possible for an ARI developer to create a new one with the same uniqueid believing the old one was destroyed when, in fact, the old one's destructor hadn't completed. This could cause the new bridge to get the old one's topic just before the topic was destroyed. When the new bridge attempted to publish a message on that topic, asterisk could either FRACK or SEGV. * The ARI bridges resource also allowed multiple bridges to be created with the same uniqueid but it kept the duplicate bridges in its app_bridges container. This created a situation where if you added two bridges with the same "bridge1" uniqueid, all operations on "bridge1" were performed on the first bridge created and the second was basically orphaned. If you attempted to delete what you thought was the second bridge, you actually deleted the first one created. Changes: * A new API `ast_bridge_topic_exists(uniqueid)` was created to determine if a topic already exists for a bridge. * `bridge_base_init()` in bridge.c and `ast_ari_bridges_create()` in resource_bridges.c now call `ast_bridge_topic_exists(uniqueid)` to check if a bridge with the requested uniqueid already exists and will fail if it does. * `bridge_register()` in bridges.c now checks the core bridges container to make sure a bridge doesn't already exist with the requested uniqueid. Although most callers of `bridge_register()` will have already called `bridge_base_init()`, which will now fail on duplicate bridges, there is no guarantee of this so we must check again. * The core bridges container allocation was changed to reject duplicate uniqueids instead of silently replacing an existing one. This is a "belt and suspenders" check. * A global mutex was added to bridge.c to prevent concurrent calls to `bridge_base_init()` and `bridge_register()`. * Even though you can no longer create multiple bridges with the same uniqueid at the same time, it's still possible that the bridge topic might be destroyed while a second bridge with the same uniqueid was trying to use it. To address this, the bridging core now increments the reference count on bridge->topic when a bridge is created and decrements it when the bridge is destroyed. * `bridge_create_common()` in res_stasis.c now checks the stasis app_bridges container to make sure a bridge with the requested uniqueid doesn't already exist. This may seem like overkill but there are so many entrypoints to bridge creation that we need to be safe and catch issues as soon in the process as possible. * The stasis app_bridges container allocation was changed to reject duplicate uniqueids instead of adding them. This is a "belt and suspenders" check. * The `bridge show all` CLI command now shows the bridge name as well as the bridge id. * Response code 409 "Conflict" was added as a possible response from the ARI bridge create resources to signal that a bridge with the requested uniqueid already exists. * Additional debugging was added to multiple bridging and stasis files. Resolves: #211 (cherry picked from commit 6cf62b6032a4b6aefb808915919f3bde00e249bd) --- include/asterisk/bridge.h | 4 + include/asterisk/stasis_bridges.h | 9 +++ main/bridge.c | 117 +++++++++++++++++++++++------- main/stasis_bridges.c | 31 ++++++++ res/ari/resource_bridges.c | 35 ++++----- res/ari/resource_bridges.h | 2 +- res/res_ari_bridges.c | 2 + res/res_stasis.c | 47 +++++++++++- res/stasis/stasis_bridge.c | 11 ++- rest-api/api-docs/bridges.json | 14 +++- 10 files changed, 219 insertions(+), 53 deletions(-) diff --git a/include/asterisk/bridge.h b/include/asterisk/bridge.h index 9e5f337bb1..58f627a253 100644 --- a/include/asterisk/bridge.h +++ b/include/asterisk/bridge.h @@ -75,6 +75,10 @@ extern "C" { #include "asterisk/dsp.h" #include "asterisk/uuid.h" +/* Macros to assist debugging */ +#define BRIDGE_PRINTF_SPEC "%s(%s)(%p)" +#define BRIDGE_PRINTF_VARS(bridge) S_OR((bridge)->uniqueid, ""), S_OR((bridge)->name, ""), (bridge) + struct a02_container; struct ast_bridge_technology; struct ast_bridge; diff --git a/include/asterisk/stasis_bridges.h b/include/asterisk/stasis_bridges.h index a064f0271c..dba76ae979 100644 --- a/include/asterisk/stasis_bridges.h +++ b/include/asterisk/stasis_bridges.h @@ -78,6 +78,15 @@ struct stasis_topic *ast_bridge_topic(struct ast_bridge *bridge); */ struct stasis_topic *ast_bridge_topic_all(void); +/*! + * \brief Check if a stasis topic exists for a bridge uniqueid + * + * \param uniqueid The uniqueid to test + * \retval 1 if the topic exists + * \retval 0 if the topic does not exist + */ +int ast_bridge_topic_exists(const char *uniqueid); + /*! * \since 12 * \brief Publish the state of a bridge diff --git a/main/bridge.c b/main/bridge.c index 4073e9738b..31866305ab 100644 --- a/main/bridge.c +++ b/main/bridge.c @@ -133,6 +133,8 @@ static struct ao2_container *bridges; static AST_RWLIST_HEAD_STATIC(bridge_technologies, ast_bridge_technology); +AST_MUTEX_DEFINE_STATIC(bridge_init_lock); + static unsigned int optimization_id; /* Initial starting point for the bridge array of channels */ @@ -330,6 +332,8 @@ void bridge_dissolve(struct ast_bridge *bridge, int cause) }; if (bridge->dissolved) { + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": already dissolved\n", + BRIDGE_PRINTF_VARS(bridge)); return; } bridge->dissolved = 1; @@ -339,16 +343,22 @@ void bridge_dissolve(struct ast_bridge *bridge, int cause) } bridge->cause = cause; - ast_debug(1, "Bridge %s: dissolving bridge with cause %d(%s)\n", - bridge->uniqueid, cause, ast_cause2str(cause)); + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": dissolving with cause %d(%s)\n", + BRIDGE_PRINTF_VARS(bridge), cause, ast_cause2str(cause)); AST_LIST_TRAVERSE(&bridge->channels, bridge_channel, entry) { + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": kicking channel %s\n", + BRIDGE_PRINTF_VARS(bridge), + ast_channel_name(bridge_channel->chan)); ast_bridge_channel_leave_bridge(bridge_channel, BRIDGE_CHANNEL_STATE_END_NO_DISSOLVE, cause); } /* Must defer dissolving bridge because it is already locked. */ ast_bridge_queue_action(bridge, &action); + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": DEFERRED_DISSOLVING queued. current refcound: %d\n", + BRIDGE_PRINTF_VARS(bridge), ao2_ref(bridge, 0)); + } /*! @@ -513,27 +523,27 @@ static struct ast_bridge_technology *find_best_technology(uint32_t capabilities, AST_RWLIST_RDLOCK(&bridge_technologies); AST_RWLIST_TRAVERSE(&bridge_technologies, current, entry) { if (current->suspended) { - ast_debug(1, "Bridge technology %s is suspended. Skipping.\n", + ast_debug(2, "Bridge technology %s is suspended. Skipping.\n", current->name); continue; } if (!(current->capabilities & capabilities)) { - ast_debug(1, "Bridge technology %s does not have any capabilities we want.\n", + ast_debug(2, "Bridge technology %s does not have any capabilities we want.\n", current->name); continue; } if (best && current->preference <= best->preference) { - ast_debug(1, "Bridge technology %s has less preference than %s (%u <= %u). Skipping.\n", + ast_debug(2, "Bridge technology %s has less preference than %s (%u <= %u). Skipping.\n", current->name, best->name, current->preference, best->preference); continue; } if (current->compatible && !current->compatible(bridge)) { - ast_debug(1, "Bridge technology %s is not compatible with properties of existing bridge.\n", + ast_debug(2, "Bridge technology %s is not compatible with properties of existing bridge.\n", current->name); continue; } if (!ast_module_running_ref(current->mod)) { - ast_debug(1, "Bridge technology %s is not running, skipping.\n", current->name); + ast_debug(2, "Bridge technology %s is not running, skipping.\n", current->name); continue; } if (best) { @@ -650,8 +660,8 @@ static void destroy_bridge(void *obj) { struct ast_bridge *bridge = obj; - ast_debug(1, "Bridge %s: actually destroying %s bridge, nobody wants it anymore\n", - bridge->uniqueid, bridge->v_table->name); + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": actually destroying %s bridge, nobody wants it anymore\n", + BRIDGE_PRINTF_VARS(bridge), bridge->v_table->name); if (bridge->construction_completed) { bridge_topics_destroy(bridge); @@ -693,18 +703,40 @@ static void destroy_bridge(void *obj) cleanup_video_mode(bridge); + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": destroyed\n", + BRIDGE_PRINTF_VARS(bridge)); ast_string_field_free_memory(bridge); ao2_cleanup(bridge->current_snapshot); + bridge->current_snapshot = NULL; } struct ast_bridge *bridge_register(struct ast_bridge *bridge) { if (bridge) { + SCOPED_MUTEX(lock, &bridge_init_lock); + /* + * Although bridge_base_init() should have already checked for + * an existing bridge with the same uniqueid, bridge_base_init() + * and bridge_register() are two separate public APIs so we need + * to check again here. + */ + struct ast_bridge *existing = ast_bridge_find_by_id(bridge->uniqueid); + if (existing) { + ast_log(LOG_WARNING, "Bridge " BRIDGE_PRINTF_SPEC ": already registered\n", + BRIDGE_PRINTF_VARS(bridge)); + ao2_ref(existing, -1); + ast_bridge_destroy(bridge, 0); + return NULL; + } + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": registering\n", + BRIDGE_PRINTF_VARS(bridge)); bridge->construction_completed = 1; ast_bridge_lock(bridge); ast_bridge_publish_state(bridge); ast_bridge_unlock(bridge); if (!ao2_link(bridges, bridge)) { + ast_log(LOG_WARNING, "Bridge " BRIDGE_PRINTF_SPEC ": failed to link\n", + BRIDGE_PRINTF_VARS(bridge)); ast_bridge_destroy(bridge, 0); bridge = NULL; } @@ -756,29 +788,45 @@ struct ast_bridge *bridge_base_init(struct ast_bridge *self, uint32_t capabiliti return NULL; } - if (!ast_strlen_zero(id)) { - ast_string_field_set(self, uniqueid, id); - } else { - ast_uuid_generate_str(uuid_hold, AST_UUID_STR_LEN); - ast_string_field_set(self, uniqueid, uuid_hold); + { + /* + * We need to ensure that another bridge with the same uniqueid + * doesn't get created before the previous bridge's destructor + * has run and deleted the existing topic. + */ + SCOPED_MUTEX(lock, &bridge_init_lock); + if (!ast_strlen_zero(id)) { + if (ast_bridge_topic_exists(id)) { + ast_log(LOG_WARNING, "Bridge " BRIDGE_PRINTF_SPEC ": already registered\n", + BRIDGE_PRINTF_VARS(self)); + ast_bridge_destroy(self, 0); + return NULL; + } + ast_string_field_set(self, uniqueid, id); + } else { + ast_uuid_generate_str(uuid_hold, AST_UUID_STR_LEN); + ast_string_field_set(self, uniqueid, uuid_hold); + } + if (!(flags & AST_BRIDGE_FLAG_INVISIBLE)) { + if (bridge_topics_init(self) != 0) { + ast_log(LOG_WARNING, "Bridge " BRIDGE_PRINTF_SPEC ": Could not initialize topics\n", + BRIDGE_PRINTF_VARS(self)); + ao2_ref(self, -1); + return NULL; + } + } } + ast_string_field_set(self, creator, creator); if (!ast_strlen_zero(creator)) { ast_string_field_set(self, name, name); } + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": base_init\n", + BRIDGE_PRINTF_VARS(self)); ast_set_flag(&self->feature_flags, flags); self->allowed_capabilities = capabilities; - if (!(flags & AST_BRIDGE_FLAG_INVISIBLE)) { - if (bridge_topics_init(self) != 0) { - ast_log(LOG_WARNING, "Bridge %s: Could not initialize topics\n", - self->uniqueid); - ao2_ref(self, -1); - return NULL; - } - } - /* Use our helper function to find the "best" bridge technology. */ self->technology = find_best_technology(capabilities, self); if (!self->technology) { @@ -814,6 +862,8 @@ struct ast_bridge *bridge_base_init(struct ast_bridge *self, uint32_t capabiliti } self->creationtime = ast_tvnow(); + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": base_init complete\n", + BRIDGE_PRINTF_VARS(self)); return self; } @@ -829,6 +879,8 @@ struct ast_bridge *bridge_base_init(struct ast_bridge *self, uint32_t capabiliti */ static void bridge_base_destroy(struct ast_bridge *self) { + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": destroying bridge (noop)\n", + BRIDGE_PRINTF_VARS(self)); } /*! @@ -840,7 +892,11 @@ static void bridge_base_destroy(struct ast_bridge *self) */ static void bridge_base_dissolving(struct ast_bridge *self) { + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": unlinking bridge. Refcount: %d\n", + BRIDGE_PRINTF_VARS(self), ao2_ref(self, 0)); ao2_unlink(bridges, self); + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": unlinked bridge. Refcount: %d\n", + BRIDGE_PRINTF_VARS(self), ao2_ref(self, 0)); } /*! @@ -952,11 +1008,15 @@ struct ast_bridge *ast_bridge_base_new(uint32_t capabilities, unsigned int flags int ast_bridge_destroy(struct ast_bridge *bridge, int cause) { - ast_debug(1, "Bridge %s: telling all channels to leave the party\n", bridge->uniqueid); + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": destroying. current refcount: %d\n", + BRIDGE_PRINTF_VARS(bridge), ao2_ref(bridge, 0)); ast_bridge_lock(bridge); bridge_dissolve(bridge, cause); ast_bridge_unlock(bridge); + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": unreffing. current refcount: %d\n", + BRIDGE_PRINTF_VARS(bridge), ao2_ref(bridge, 0)); + ao2_ref(bridge, -1); return 0; @@ -5044,8 +5104,8 @@ static char *complete_bridge_live(const char *word) static char *handle_bridge_show_all(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a) { -#define FORMAT_HDR "%-36s %5s %-15s %-15s %s\n" -#define FORMAT_ROW "%-36s %5u %-15s %-15s %s\n" +#define FORMAT_HDR "%-36s %-36s %5s %-15s %-15s %s\n" +#define FORMAT_ROW "%-36s %-36s %5u %-15s %-15s %s\n" struct ao2_iterator iter; struct ast_bridge *bridge; @@ -5061,7 +5121,7 @@ static char *handle_bridge_show_all(struct ast_cli_entry *e, int cmd, struct ast return NULL; } - ast_cli(a->fd, FORMAT_HDR, "Bridge-ID", "Chans", "Type", "Technology", "Duration"); + ast_cli(a->fd, FORMAT_HDR, "Bridge-ID", "Name", "Chans", "Type", "Technology", "Duration"); iter = ao2_iterator_init(bridges, 0); for (; (bridge = ao2_iterator_next(&iter)); ao2_ref(bridge, -1)) { @@ -5071,7 +5131,8 @@ static char *handle_bridge_show_all(struct ast_cli_entry *e, int cmd, struct ast if (snapshot) { ast_format_duration_hh_mm_ss(ast_tvnow().tv_sec - snapshot->creationtime.tv_sec, print_time, sizeof(print_time)); ast_cli(a->fd, FORMAT_ROW, - snapshot->uniqueid, + bridge->uniqueid, + S_OR(bridge->name, ""), snapshot->num_channels, S_OR(snapshot->subclass, ""), S_OR(snapshot->technology, ""), diff --git a/main/stasis_bridges.c b/main/stasis_bridges.c index 5fcd9ae299..06b77a873a 100644 --- a/main/stasis_bridges.c +++ b/main/stasis_bridges.c @@ -196,6 +196,26 @@ struct stasis_topic *ast_bridge_topic(struct ast_bridge *bridge) return bridge->topic; } +int ast_bridge_topic_exists(const char *uniqueid) +{ + char *topic_name; + int ret; + + if (ast_strlen_zero(uniqueid)) { + return 0; + } + + ret = ast_asprintf(&topic_name, "bridge:%s", uniqueid); + if (ret < 0) { + return 0; + } + ret = stasis_topic_pool_topic_exists(bridge_topic_pool, topic_name); + ast_free(topic_name); + + return ret; +} + + /*! \brief Destructor for bridge snapshots */ static void bridge_snapshot_dtor(void *obj) { @@ -318,6 +338,7 @@ int bridge_topics_init(struct ast_bridge *bridge) if (!bridge->topic) { return -1; } + ao2_bump(bridge->topic); return 0; } @@ -328,6 +349,14 @@ void bridge_topics_destroy(struct ast_bridge *bridge) struct stasis_message *msg; ast_assert(bridge != NULL); + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": destroying topics\n", + BRIDGE_PRINTF_VARS(bridge)); + + if (!bridge->topic) { + ast_log(LOG_WARNING, "Bridge " BRIDGE_PRINTF_SPEC " topic is NULL\n", + BRIDGE_PRINTF_VARS(bridge)); + return; + } if (!bridge->current_snapshot) { bridge->current_snapshot = ast_bridge_snapshot_create(bridge); @@ -351,6 +380,8 @@ void bridge_topics_destroy(struct ast_bridge *bridge) ao2_ref(msg, -1); stasis_topic_pool_delete_topic(bridge_topic_pool, stasis_topic_name(ast_bridge_topic(bridge))); + ao2_cleanup(bridge->topic); + bridge->topic = NULL; } void ast_bridge_publish_state(struct ast_bridge *bridge) diff --git a/res/ari/resource_bridges.c b/res/ari/resource_bridges.c index b66e41cc81..fafe03aa58 100644 --- a/res/ari/resource_bridges.c +++ b/res/ari/resource_bridges.c @@ -947,13 +947,21 @@ void ast_ari_bridges_create(struct ast_variable *headers, struct ast_ari_bridges_create_args *args, struct ast_ari_response *response) { - RAII_VAR(struct ast_bridge *, bridge, stasis_app_bridge_create(args->type, args->name, args->bridge_id), ao2_cleanup); + RAII_VAR(struct ast_bridge *, bridge, NULL, ao2_cleanup); RAII_VAR(struct ast_bridge_snapshot *, snapshot, NULL, ao2_cleanup); + if (ast_bridge_topic_exists(args->bridge_id)) { + ast_ari_response_error( + response, 409, "Conflict", + "Bridge with id '%s' already exists", args->bridge_id); + return; + } + + bridge = stasis_app_bridge_create(args->type, args->name, args->bridge_id); if (!bridge) { ast_ari_response_error( response, 500, "Internal Error", - "Unable to create bridge"); + "Unable to create bridge. Possible duplicate bridge id '%s'", args->bridge_id); return; } @@ -976,26 +984,13 @@ void ast_ari_bridges_create_with_id(struct ast_variable *headers, struct ast_ari_bridges_create_with_id_args *args, struct ast_ari_response *response) { - RAII_VAR(struct ast_bridge *, bridge, find_bridge(response, args->bridge_id), ao2_cleanup); + RAII_VAR(struct ast_bridge *, bridge, NULL, ao2_cleanup); RAII_VAR(struct ast_bridge_snapshot *, snapshot, NULL, ao2_cleanup); - if (bridge) { - /* update */ - if (!ast_strlen_zero(args->name) - && strcmp(args->name, bridge->name)) { - ast_ari_response_error( - response, 500, "Internal Error", - "Changing bridge name is not implemented"); - return; - } - if (!ast_strlen_zero(args->type)) { - ast_ari_response_error( - response, 500, "Internal Error", - "Supplying a bridge type when updating a bridge is not allowed."); - return; - } - ast_ari_response_ok(response, - ast_bridge_snapshot_to_json(snapshot, stasis_app_get_sanitizer())); + if (ast_bridge_topic_exists(args->bridge_id)) { + ast_ari_response_error( + response, 409, "Conflict", + "Bridge with id '%s' already exists", args->bridge_id); return; } diff --git a/res/ari/resource_bridges.h b/res/ari/resource_bridges.h index 01d5d6070c..d494d89008 100644 --- a/res/ari/resource_bridges.h +++ b/res/ari/resource_bridges.h @@ -101,7 +101,7 @@ int ast_ari_bridges_create_with_id_parse_body( struct ast_ari_bridges_create_with_id_args *args); /*! - * \brief Create a new bridge or updates an existing one. + * \brief Create a new bridge. * * This bridge persists until it has been shut down, or Asterisk has been shut down. * diff --git a/res/res_ari_bridges.c b/res/res_ari_bridges.c index 6cb991e69f..ba649a6ded 100644 --- a/res/res_ari_bridges.c +++ b/res/res_ari_bridges.c @@ -172,6 +172,7 @@ static void ast_ari_bridges_create_cb( break; case 500: /* Internal Server Error */ case 501: /* Not Implemented */ + case 409: /* Bridge with the same bridgeId already exists */ is_valid = 1; break; default: @@ -261,6 +262,7 @@ static void ast_ari_bridges_create_with_id_cb( break; case 500: /* Internal Server Error */ case 501: /* Not Implemented */ + case 409: /* Bridge with the same bridgeId already exists */ is_valid = 1; break; default: diff --git a/res/res_stasis.c b/res/res_stasis.c index 84244c2a13..baa4f37817 100644 --- a/res/res_stasis.c +++ b/res/res_stasis.c @@ -414,6 +414,32 @@ static int bridges_compare(void *obj, void *arg, int flags) return CMP_MATCH; } +/*! AO2 sort function for bridges container */ +static int bridges_sort (const void *left, const void *right, const int flags) +{ + const struct ast_bridge *object_left = left; + const struct ast_bridge *object_right = right; + const char *right_key = right; + int cmp; + + switch (flags & OBJ_SEARCH_MASK) { + case OBJ_SEARCH_OBJECT: + right_key = object_right->uniqueid; + /* Fall through */ + case OBJ_SEARCH_KEY: + cmp = strcmp(object_left->uniqueid, right_key); + break; + case OBJ_SEARCH_PARTIAL_KEY: + cmp = strncmp(object_left->uniqueid, right_key, strlen(right_key)); + break; + default: + ast_assert(0); + cmp = 0; + break; + } + return cmp; +} + /*! * Used with app_bridges_moh and app_bridge_control, they provide links * between bridges and channels used for ARI application purposes @@ -803,10 +829,21 @@ static struct ast_bridge *bridge_create_common(const char *type, const char *nam enum ast_bridge_video_mode_type video_mode = AST_BRIDGE_VIDEO_MODE_TALKER_SRC; int send_sdp_label = 0; + ast_debug(1, "Creating bridge of type '%s' with name '%s' and id '%s'\n", + type, S_OR(name, ""), S_OR(id, "")); if (invisible) { flags |= AST_BRIDGE_FLAG_INVISIBLE; } + if (!ast_strlen_zero(id)) { + bridge = stasis_app_bridge_find_by_id(id); + if (bridge) { + ast_log(LOG_WARNING, "Bridge with id '%s' already exists\n", id); + ao2_ref(bridge, -1); + return NULL; + } + } + while ((requested_type = strsep(&requested_types, ","))) { requested_type = ast_strip(requested_type); @@ -867,7 +904,12 @@ void stasis_app_bridge_destroy(const char *bridge_id) if (!bridge) { return; } + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": destroying bridge\n", + BRIDGE_PRINTF_VARS(bridge)); + ao2_unlink(app_bridges, bridge); + ast_debug(1, "Bridge " BRIDGE_PRINTF_SPEC ": unlinked from app_bridges. current refcount: %d\n", + BRIDGE_PRINTF_VARS(bridge), ao2_ref(bridge, 0)); ast_bridge_destroy(bridge, 0); } @@ -2349,8 +2391,9 @@ static int load_module(void) APPS_NUM_BUCKETS, app_hash, NULL, app_compare); app_controls = ao2_container_alloc_hash(AO2_ALLOC_OPT_LOCK_MUTEX, 0, CONTROLS_NUM_BUCKETS, control_hash, NULL, control_compare); - app_bridges = ao2_container_alloc_hash(AO2_ALLOC_OPT_LOCK_MUTEX, 0, - BRIDGES_NUM_BUCKETS, bridges_hash, NULL, bridges_compare); + app_bridges = ao2_container_alloc_hash(AO2_ALLOC_OPT_LOCK_MUTEX, + AO2_CONTAINER_ALLOC_OPT_DUPS_REJECT, + BRIDGES_NUM_BUCKETS, bridges_hash, bridges_sort, bridges_compare); app_bridges_moh = ao2_container_alloc_hash( AO2_ALLOC_OPT_LOCK_MUTEX, 0, 37, bridges_channel_hash_fn, NULL, bridges_channel_compare); diff --git a/res/stasis/stasis_bridge.c b/res/stasis/stasis_bridge.c index f48f60cab9..7ada83e15d 100644 --- a/res/stasis/stasis_bridge.c +++ b/res/stasis/stasis_bridge.c @@ -297,8 +297,10 @@ static void bridge_stasis_pull(struct ast_bridge *self, struct ast_bridge_channe struct ast_bridge *bridge_stasis_new(uint32_t capabilities, unsigned int flags, const char *name, const char *id, enum ast_bridge_video_mode_type video_mode, unsigned int send_sdp_label) { - void *bridge; + struct ast_bridge *bridge; + ast_debug(1, "Creating Stasis bridge '%s' with id '%s'\n", + S_OR(name, ""), S_OR(id, "")); bridge = bridge_alloc(sizeof(struct ast_bridge), &bridge_stasis_v_table); bridge = bridge_base_init(bridge, capabilities, flags, "Stasis", name, id); if (!bridge) { @@ -322,6 +324,13 @@ struct ast_bridge *bridge_stasis_new(uint32_t capabilities, unsigned int flags, } bridge = bridge_register(bridge); + if (!bridge) { + ast_log(LOG_ERROR, "Failed to register Stasis bridge %s(%s)\n", + S_OR(id, ""), S_OR(name, "")); + return NULL; + } + ast_debug(1, "Created Stasis bridge " BRIDGE_PRINTF_SPEC "\n", + BRIDGE_PRINTF_VARS(bridge)); return bridge; } diff --git a/rest-api/api-docs/bridges.json b/rest-api/api-docs/bridges.json index f7869e820a..4cd15b9794 100644 --- a/rest-api/api-docs/bridges.json +++ b/rest-api/api-docs/bridges.json @@ -61,6 +61,12 @@ "allowMultiple": false, "dataType": "string" } + ], + "errorResponses": [ + { + "code": 409, + "reason": "Bridge with the same bridgeId already exists" + } ] } ] @@ -74,7 +80,7 @@ "since": [ "12.2.0" ], - "summary": "Create a new bridge or updates an existing one.", + "summary": "Create a new bridge.", "notes": "This bridge persists until it has been shut down, or Asterisk has been shut down.", "nickname": "createWithId", "responseClass": "Bridge", @@ -103,6 +109,12 @@ "allowMultiple": false, "dataType": "string" } + ], + "errorResponses": [ + { + "code": 409, + "reason": "Bridge with the same bridgeId already exists" + } ] }, {