Misc bridge code improvements
authorRichard Mudgett <rmudgett@digium.com>
Fri, 25 Jan 2013 20:00:21 +0000 (20:00 +0000)
committerRichard Mudgett <rmudgett@digium.com>
Fri, 25 Jan 2013 20:00:21 +0000 (20:00 +0000)
* Made multiplexed_bridge_destroy() check if anything to destroy and
cleared bridge_pvt pointer after destruction.

* Made multiplexed_add_or_remove() handling of the chans array simpler.

* Extracted bridge_channel_poke().

* Simplified bridge_array_remove() handling of the bridge->array[].  The
array does not have a NULL sentinel pointer.

* Made ast_bridge_new() not create a temporary bridge just to see if it
can be done.  Only need to check if there is an appropriate bridge tech
available.

* Made ast_bridge_new() clean up on allocation failures.

* Made destroy_bridge() free resources in the opposite order of creation.

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@380109 65c4cc65-6c06-0410-ace0-fbb531ad65f3

bridges/bridge_multiplexed.c
main/bridging.c

index c251e2d..2f8dd97 100644 (file)
@@ -178,7 +178,12 @@ static void multiplexed_nudge(struct multiplexed_thread *multiplexed_thread)
 /*! \brief Destroy function which unreserves/unreferences/removes a multiplexed thread structure */
 static int multiplexed_bridge_destroy(struct ast_bridge *bridge)
 {
-       struct multiplexed_thread *multiplexed_thread = bridge->bridge_pvt;
+       struct multiplexed_thread *multiplexed_thread;
+
+       multiplexed_thread = bridge->bridge_pvt;
+       if (!multiplexed_thread) {
+               return -1;
+       }
 
        ao2_lock(multiplexed_threads);
 
@@ -194,6 +199,7 @@ static int multiplexed_bridge_destroy(struct ast_bridge *bridge)
        ao2_unlock(multiplexed_threads);
 
        ao2_ref(multiplexed_thread, -1);
+       bridge->bridge_pvt = NULL;
 
        return 0;
 }
@@ -270,27 +276,37 @@ static void *multiplexed_thread_function(void *data)
 /*! \brief Helper function which adds or removes a channel and nudges the thread */
 static void multiplexed_add_or_remove(struct multiplexed_thread *multiplexed_thread, struct ast_channel *chan, int add)
 {
-       int i, removed = 0;
+       int idx;
        pthread_t thread = AST_PTHREADT_NULL;
 
        ao2_lock(multiplexed_thread);
 
        multiplexed_nudge(multiplexed_thread);
 
-       for (i = 0; i < MULTIPLEXED_MAX_CHANNELS; i++) {
-               if (multiplexed_thread->chans[i] == chan) {
+       for (idx = 0; idx < ARRAY_LEN(multiplexed_thread->chans); ++idx) {
+               if (multiplexed_thread->chans[idx] == chan) {
                        if (!add) {
-                               multiplexed_thread->chans[i] = NULL;
-                               multiplexed_thread->service_count--;
-                               removed = 1;
+                               memmove(multiplexed_thread->chans + idx,
+                                       multiplexed_thread->chans + idx + 1,
+                                       sizeof(struct ast_channel *) * (ARRAY_LEN(multiplexed_thread->chans) - (idx + 1)));
+                               multiplexed_thread->chans[ARRAY_LEN(multiplexed_thread->chans) - 1] = NULL;
+                               --multiplexed_thread->service_count;
                        }
                        break;
-               } else if (!multiplexed_thread->chans[i] && add) {
-                       multiplexed_thread->chans[i] = chan;
-                       multiplexed_thread->service_count++;
+               }
+               if (!multiplexed_thread->chans[idx]) {
+                       if (add) {
+                               multiplexed_thread->chans[idx] = chan;
+                               ++multiplexed_thread->service_count;
+                       }
                        break;
                }
        }
+       if (ARRAY_LEN(multiplexed_thread->chans) == idx && add) {
+               ast_log(LOG_ERROR, "Could not add channel %s to multiplexed thread %p.  Array not large enough.\n",
+                       ast_channel_name(chan), multiplexed_thread);
+               ast_assert(0);
+       }
 
        if (multiplexed_thread->service_count && multiplexed_thread->thread == AST_PTHREADT_NULL) {
                ao2_ref(multiplexed_thread, +1);
@@ -299,11 +315,11 @@ static void multiplexed_add_or_remove(struct multiplexed_thread *multiplexed_thr
                        ast_log(LOG_WARNING, "Failed to create the bridge thread for multiplexed thread '%p', trying next time\n",
                                multiplexed_thread);
                }
-       } else if (!multiplexed_thread->service_count && multiplexed_thread->thread != AST_PTHREADT_NULL) {
+       } else if (!multiplexed_thread->service_count
+               && multiplexed_thread->thread != AST_PTHREADT_NULL
+               && multiplexed_thread->thread != AST_PTHREADT_STOP) {
                thread = multiplexed_thread->thread;
                multiplexed_thread->thread = AST_PTHREADT_STOP;
-       } else if (!add && removed) {
-               memmove(multiplexed_thread->chans + i, multiplexed_thread->chans + i + 1, sizeof(struct ast_channel *) * (MULTIPLEXED_MAX_CHANNELS - (i + 1)));
        }
 
        ao2_unlock(multiplexed_thread);
index 9753a9d..20cfb79 100644 (file)
@@ -118,6 +118,14 @@ int ast_bridge_technology_unregister(struct ast_bridge_technology *technology)
        return current ? 0 : -1;
 }
 
+static void bridge_channel_poke(struct ast_bridge_channel *bridge_channel)
+{
+       ao2_lock(bridge_channel);
+       pthread_kill(bridge_channel->thread, SIGURG);
+       ast_cond_signal(&bridge_channel->cond);
+       ao2_unlock(bridge_channel);
+}
+
 /*! \note This function assumes the bridge_channel is locked. */
 static void ast_bridge_change_state_nolock(struct ast_bridge_channel *bridge_channel, enum ast_bridge_channel_state new_state)
 {
@@ -211,7 +219,7 @@ static void bridge_array_add(struct ast_bridge *bridge, struct ast_channel *chan
  */
 static void bridge_array_remove(struct ast_bridge *bridge, struct ast_channel *chan)
 {
-       int i;
+       int idx;
 
        /* We have to make sure the bridge thread is not using the bridge array before messing with it */
        while (bridge->waiting) {
@@ -219,12 +227,12 @@ static void bridge_array_remove(struct ast_bridge *bridge, struct ast_channel *c
                sched_yield();
        }
 
-       for (i = 0; i < bridge->array_num; i++) {
-               if (bridge->array[i] == chan) {
-                       bridge->array[i] = (bridge->array[(bridge->array_num - 1)] != chan ? bridge->array[(bridge->array_num - 1)] : NULL);
-                       bridge->array[(bridge->array_num - 1)] = NULL;
-                       bridge->array_num--;
-                       ast_debug(1, "Removed channel %p from bridge array on %p, new count is %d\n", chan, bridge, (int)bridge->array_num);
+       for (idx = 0; idx < bridge->array_num; ++idx) {
+               if (bridge->array[idx] == chan) {
+                       --bridge->array_num;
+                       bridge->array[idx] = bridge->array[bridge->array_num];
+                       ast_debug(1, "Removed channel %p from bridge array on %p, new count is %d\n",
+                               chan, bridge, (int) bridge->array_num);
                        break;
                }
        }
@@ -515,16 +523,16 @@ static void destroy_bridge(void *obj)
                }
        }
 
-       /* We are no longer using the bridge technology so decrement the module reference count on it */
-       ast_module_unref(bridge->technology->mod);
+       cleanup_video_mode(bridge);
 
-       /* Last but not least clean up the features configuration */
+       /* Clean up the features configuration */
        ast_bridge_features_cleanup(&bridge->features);
 
+       /* We are no longer using the bridge technology so decrement the module reference count on it */
+       ast_module_unref(bridge->technology->mod);
+
        /* Drop the array of channels */
        ast_free(bridge->array);
-
-       cleanup_video_mode(bridge);
 }
 
 struct ast_bridge *ast_bridge_new(uint32_t capabilities, int flags)
@@ -534,13 +542,10 @@ struct ast_bridge *ast_bridge_new(uint32_t capabilities, int flags)
 
        /* If we need to be a smart bridge see if we can move between 1to1 and multimix bridges */
        if (flags & AST_BRIDGE_FLAG_SMART) {
-               struct ast_bridge *other_bridge;
-
-               if (!(other_bridge = ast_bridge_new((capabilities & AST_BRIDGE_CAPABILITY_1TO1MIX) ? AST_BRIDGE_CAPABILITY_MULTIMIX : AST_BRIDGE_CAPABILITY_1TO1MIX, 0))) {
+               if (!ast_bridge_check((capabilities & AST_BRIDGE_CAPABILITY_1TO1MIX)
+                       ? AST_BRIDGE_CAPABILITY_MULTIMIX : AST_BRIDGE_CAPABILITY_1TO1MIX)) {
                        return NULL;
                }
-
-               ast_bridge_destroy(other_bridge);
        }
 
        /*
@@ -558,7 +563,9 @@ struct ast_bridge *ast_bridge_new(uint32_t capabilities, int flags)
        }
 
        /* We have everything we need to create this bridge... so allocate the memory, link things together, and fire her up! */
-       if (!(bridge = ao2_alloc(sizeof(*bridge), destroy_bridge))) {
+       bridge = ao2_alloc(sizeof(*bridge), destroy_bridge);
+       if (!bridge) {
+               ast_module_unref(bridge_technology->mod);
                return NULL;
        }
 
@@ -566,7 +573,11 @@ struct ast_bridge *ast_bridge_new(uint32_t capabilities, int flags)
        bridge->thread = AST_PTHREADT_NULL;
 
        /* Create an array of pointers for the channels that will be joining us */
-       bridge->array = ast_calloc(BRIDGE_ARRAY_START, sizeof(struct ast_channel*));
+       bridge->array = ast_malloc(BRIDGE_ARRAY_START * sizeof(*bridge->array));
+       if (!bridge->array) {
+               ao2_ref(bridge, -1);
+               return NULL;
+       }
        bridge->array_size = BRIDGE_ARRAY_START;
 
        ast_set_flag(&bridge->feature_flags, flags);
@@ -783,10 +794,7 @@ static int smart_bridge_operation(struct ast_bridge *bridge, struct ast_bridge_c
                }
 
                /* Fourth we tell them to wake up so they become aware that the above has happened */
-               pthread_kill(bridge_channel2->thread, SIGURG);
-               ao2_lock(bridge_channel2);
-               ast_cond_signal(&bridge_channel2->cond);
-               ao2_unlock(bridge_channel2);
+               bridge_channel_poke(bridge_channel2);
        }
 
        /* Now that all the channels have been moved over we need to get rid of all the information the old technology may have left around */
@@ -1383,10 +1391,7 @@ int ast_bridge_merge(struct ast_bridge *bridge0, struct ast_bridge *bridge1)
                }
 
                /* Poke the bridge channel, this will cause it to wake up and execute the proper threading model for the new bridge it is in */
-               pthread_kill(bridge_channel->thread, SIGURG);
-               ao2_lock(bridge_channel);
-               ast_cond_signal(&bridge_channel->cond);
-               ao2_unlock(bridge_channel);
+               bridge_channel_poke(bridge_channel);
        }
 
        ast_debug(1, "Merged channels from bridge %p into bridge %p\n", bridge1, bridge0);