Simplify interval hooks since there is only one bridge threading model now.

* Convert interval timers to use the ast_waitfor_nandfds() timeout.

* Remove bridge channel action for intervals.  Now the main loop handles
running interval hooks.

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

diff --git a/include/asterisk/bridging_channel_internal.h b/include/asterisk/bridging_channel_internal.h
index 9bc9b11f9e253fdff8aecb85b7b481c0f4fa30d5..cbfa20aa6b3448d1e1e3b2a18abbf8ffab78d281 100644 (file)
--- a/include/asterisk/bridging_channel_internal.h
+++ b/include/asterisk/bridging_channel_internal.h
@@ -40,8 +40,6 @@
 enum bridge_channel_action_type {
        /*! Bridged channel is to detect a feature hook */
        BRIDGE_CHANNEL_ACTION_FEATURE,
-       /*! Bridged channel is to act on an interval hook */
-       BRIDGE_CHANNEL_ACTION_INTERVAL,
        /*! Bridged channel is to send a DTMF stream out */
        BRIDGE_CHANNEL_ACTION_DTMF_STREAM,
        /*! Bridged channel is to indicate talking start */

diff --git a/include/asterisk/bridging_features.h b/include/asterisk/bridging_features.h
index acdd6e0ba53160c21d3c51c595c20c1ec878c8b7..dacb6c698fce4caaf6b2a1b6d240f43c47c13cda 100644 (file)
--- a/include/asterisk/bridging_features.h
+++ b/include/asterisk/bridging_features.h
@@ -226,8 +226,6 @@ struct ast_bridge_hook_timer {
        struct ast_bridge_hook_timer_parms timer;
 };
 
-#define BRIDGE_FEATURES_INTERVAL_RATE 10
-
 /*!
  * \brief Structure that contains features information
  */
@@ -238,8 +236,6 @@ struct ast_bridge_features {
        struct ao2_container *other_hooks;
        /*! Attached interval hooks */
        struct ast_heap *interval_hooks;
-       /*! Used to determine when interval based features should be checked */
-       struct ast_timer *interval_timer;
        /*! Limits feature data */
        struct ast_bridge_features_limits *limits;
        /*! Feature flags that are enabled */

diff --git a/main/bridging.c b/main/bridging.c
index bc1d36d83827b2b6c1a5106213c0d4df431c95b9..dda0732365748bf5e31add109be2f280f699133c 100644 (file)
--- a/main/bridging.c
+++ b/main/bridging.c
@@ -3641,14 +3641,6 @@ int ast_bridge_interval_hook(struct ast_bridge_features *features,
                return -1;
        }
 
-       if (!features->interval_timer) {
-               if (!(features->interval_timer = ast_timer_open())) {
-                       ast_log(LOG_ERROR, "Failed to open a timer when adding a timed bridging feature.\n");
-                       return -1;
-               }
-               ast_timer_set_rate(features->interval_timer, BRIDGE_FEATURES_INTERVAL_RATE);
-       }
-
        /* Allocate new hook and setup it's various variables */
        hook = (struct ast_bridge_hook_timer *) bridge_hook_generic(sizeof(*hook), callback,
                hook_pvt, destructor, remove_flags);
@@ -3922,11 +3914,6 @@ void ast_bridge_features_cleanup(struct ast_bridge_features *features)
                features->interval_hooks = ast_heap_destroy(features->interval_hooks);
        }
 
-       if (features->interval_timer) {
-               ast_timer_close(features->interval_timer);
-               features->interval_timer = NULL;
-       }
-
        /* If the features contains a limits pvt, destroy that as well. */
        if (features->limits) {
                ast_bridge_features_limits_destroy(features->limits);

diff --git a/main/bridging_channel.c b/main/bridging_channel.c
index e7e7c257979e04d5f1926687038abf574fd8b7ac..385545c2c85445b8f888b4e52372019ee67aaa61 100644 (file)
--- a/main/bridging_channel.c
+++ b/main/bridging_channel.c
@@ -698,20 +698,6 @@ int ast_bridge_channel_write_park(struct ast_bridge_channel *bridge_channel, con
                bridge_channel, parkee_uuid, parker_uuid, app_data);
 }
 
-static int bridge_channel_interval_ready(struct ast_bridge_channel *bridge_channel)
-{
-       struct ast_bridge_features *features = bridge_channel->features;
-       struct ast_bridge_hook_timer *hook;
-       int ready;
-
-       ast_heap_wrlock(features->interval_hooks);
-       hook = ast_heap_peek(features->interval_hooks, 1);
-       ready = hook && ast_tvdiff_ms(hook->timer.trip_time, ast_tvnow()) <= 0;
-       ast_heap_unlock(features->interval_hooks);
-
-       return ready;
-}
-
 int ast_bridge_notify_talking(struct ast_bridge_channel *bridge_channel, int started_talking)
 {
        struct ast_frame action = {
@@ -833,15 +819,26 @@ static void bridge_channel_unsuspend(struct ast_bridge_channel *bridge_channel)
        ast_bridge_unlock(bridge_channel->bridge);
 }
 
-/*! \brief Internal function that activates interval hooks on a bridge channel */
-static void bridge_channel_interval(struct ast_bridge_channel *bridge_channel)
+/*!
+ * \internal
+ * \brief Handle bridge channel interval expiration.
+ * \since 12.0.0
+ *
+ * \param bridge_channel Channel to run expired intervals on.
+ *
+ * \return Nothing
+ */
+static void bridge_channel_handle_interval(struct ast_bridge_channel *bridge_channel)
 {
+       struct ast_heap *interval_hooks;
        struct ast_bridge_hook_timer *hook;
        struct timeval start;
+       int hook_run = 0;
 
-       ast_heap_wrlock(bridge_channel->features->interval_hooks);
+       interval_hooks = bridge_channel->features->interval_hooks;
+       ast_heap_wrlock(interval_hooks);
        start = ast_tvnow();
-       while ((hook = ast_heap_peek(bridge_channel->features->interval_hooks, 1))) {
+       while ((hook = ast_heap_peek(interval_hooks, 1))) {
                int interval;
                unsigned int execution_time;
 
@@ -851,17 +848,22 @@ static void bridge_channel_interval(struct ast_bridge_channel *bridge_channel)
                        break;
                }
                ao2_ref(hook, +1);
-               ast_heap_unlock(bridge_channel->features->interval_hooks);
+               ast_heap_unlock(interval_hooks);
+
+               if (!hook_run) {
+                       hook_run = 1;
+                       bridge_channel_suspend(bridge_channel);
+                       ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
+               }
 
                ast_debug(1, "Executing hook %p on %p(%s)\n",
                        hook, bridge_channel, ast_channel_name(bridge_channel->chan));
                interval = hook->generic.callback(bridge_channel->bridge, bridge_channel,
                        hook->generic.hook_pvt);
 
-               ast_heap_wrlock(bridge_channel->features->interval_hooks);
-               if (ast_heap_peek(bridge_channel->features->interval_hooks,
-                       hook->timer.heap_index) != hook
-                       || !ast_heap_remove(bridge_channel->features->interval_hooks, hook)) {
+               ast_heap_wrlock(interval_hooks);
+               if (ast_heap_peek(interval_hooks, hook->timer.heap_index) != hook
+                       || !ast_heap_remove(interval_hooks, hook)) {
                        /* Interval hook is already removed from the bridge_channel. */
                        ao2_ref(hook, -1);
                        continue;
@@ -898,12 +900,17 @@ static void bridge_channel_interval(struct ast_bridge_channel *bridge_channel)
                hook->timer.trip_time = ast_tvadd(start, ast_samp2tv(hook->timer.interval - execution_time, 1000));
                hook->timer.seqno = ast_atomic_fetchadd_int((int *) &bridge_channel->features->interval_sequence, +1);
 
-               if (ast_heap_push(bridge_channel->features->interval_hooks, hook)) {
+               if (ast_heap_push(interval_hooks, hook)) {
                        /* Could not push the hook back onto the heap. */
                        ao2_ref(hook, -1);
                }
        }
-       ast_heap_unlock(bridge_channel->features->interval_hooks);
+       ast_heap_unlock(interval_hooks);
+
+       if (hook_run) {
+               ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
+               bridge_channel_unsuspend(bridge_channel);
+       }
 }
 
 static int bridge_channel_write_dtmf_stream(struct ast_bridge_channel *bridge_channel, const char *dtmf)
@@ -1138,13 +1145,6 @@ static void bridge_channel_attended_transfer(struct ast_bridge_channel *bridge_c
 static void bridge_channel_handle_action(struct ast_bridge_channel *bridge_channel, struct ast_frame *action)
 {
        switch (action->subclass.integer) {
-       case BRIDGE_CHANNEL_ACTION_INTERVAL:
-               bridge_channel_suspend(bridge_channel);
-               ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
-               bridge_channel_interval(bridge_channel);
-               ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
-               bridge_channel_unsuspend(bridge_channel);
-               break;
        case BRIDGE_CHANNEL_ACTION_FEATURE:
                bridge_channel_suspend(bridge_channel);
                ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
@@ -1381,6 +1381,9 @@ int bridge_channel_push(struct ast_bridge_channel *bridge_channel)
        pbx_builtin_setvar_helper(bridge_channel->chan, "BLINDTRANSFER", NULL);
        pbx_builtin_setvar_helper(bridge_channel->chan, "ATTENDEDTRANSFER", NULL);
 
+       /* Wake up the bridge channel thread to reevaluate any interval timers. */
+       ast_queue_frame(bridge_channel->chan, &ast_null_frame);
+
        bridge->reconfigured = 1;
        return 0;
 }
@@ -1521,36 +1524,6 @@ static void bridge_channel_handle_write(struct ast_bridge_channel *bridge_channe
        ast_frfree(fr);
 }
 
-/*!
- * \internal
- * \brief Handle bridge channel interval expiration.
- * \since 12.0.0
- *
- * \param bridge_channel Channel to check interval on.
- *
- * \return Nothing
- */
-static void bridge_channel_handle_interval(struct ast_bridge_channel *bridge_channel)
-{
-       struct ast_timer *interval_timer;
-
-       interval_timer = bridge_channel->features->interval_timer;
-       if (interval_timer) {
-               if (ast_wait_for_input(ast_timer_fd(interval_timer), 0) == 1) {
-                       ast_timer_ack(interval_timer, 1);
-                       if (bridge_channel_interval_ready(bridge_channel)) {
-/* BUGBUG since this is now only run by the channel thread, there is no need to queue the action once this intervals become a first class wait item in bridge_channel_wait(). */
-                               struct ast_frame interval_action = {
-                                       .frametype = AST_FRAME_BRIDGE_ACTION,
-                                       .subclass.integer = BRIDGE_CHANNEL_ACTION_INTERVAL,
-                               };
-
-                               ast_bridge_channel_queue_frame(bridge_channel, &interval_action);
-                       }
-               }
-       }
-}
-
 /*! \brief Internal function to handle DTMF from a channel */
 static struct ast_frame *bridge_handle_dtmf(struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
 {
@@ -1636,6 +1609,39 @@ static void bridge_handle_trip(struct ast_bridge_channel *bridge_channel)
        ast_frfree(frame);
 }
 
+/*!
+ * \internal
+ * \brief Determine how long till the next timer interval.
+ * \since 12.0.0
+ *
+ * \param bridge_channel Channel to determine how long can wait.
+ *
+ * \retval ms Number of milliseconds to wait.
+ * \retval -1 to wait forever.
+ */
+static int bridge_channel_next_interval(struct ast_bridge_channel *bridge_channel)
+{
+       struct ast_heap *interval_hooks = bridge_channel->features->interval_hooks;
+       struct ast_bridge_hook_timer *hook;
+       int ms;
+
+       ast_heap_wrlock(interval_hooks);
+       hook = ast_heap_peek(interval_hooks, 1);
+       if (hook) {
+               ms = ast_tvdiff_ms(hook->timer.trip_time, ast_tvnow());
+               if (ms < 0) {
+                       /* Expire immediately.  An interval hook is ready to run. */
+                       ms = 0;
+               }
+       } else {
+               /* No hook so wait forever. */
+               ms = -1;
+       }
+       ast_heap_unlock(interval_hooks);
+
+       return ms;
+}
+
 /*!
  * \internal
  * \brief Wait for something to happen on the bridge channel and handle it.
@@ -1649,7 +1655,7 @@ static void bridge_handle_trip(struct ast_bridge_channel *bridge_channel)
  */
 static void bridge_channel_wait(struct ast_bridge_channel *bridge_channel)
 {
-       int ms = -1;
+       int ms;
        int outfd;
        struct ast_channel *chan;
 
@@ -1669,7 +1675,7 @@ static void bridge_channel_wait(struct ast_bridge_channel *bridge_channel)
                bridge_channel->waiting = 1;
                ast_bridge_channel_unlock(bridge_channel);
                outfd = -1;
-/* BUGBUG need to make the next expiring active interval setup ms timeout rather than holding up the chan reads. */
+               ms = bridge_channel_next_interval(bridge_channel);
                chan = ast_waitfor_nandfds(&bridge_channel->chan, 1,
                        &bridge_channel->alert_pipe[0], 1, NULL, &outfd, &ms);
                bridge_channel->waiting = 0;
@@ -1686,10 +1692,12 @@ static void bridge_channel_wait(struct ast_bridge_channel *bridge_channel)
                if (!bridge_channel->suspended
                        && bridge_channel->state == AST_BRIDGE_CHANNEL_STATE_WAIT) {
                        if (chan) {
-                               bridge_channel_handle_interval(bridge_channel);
                                bridge_handle_trip(bridge_channel);
                        } else if (-1 < outfd) {
                                bridge_channel_handle_write(bridge_channel);
+                       } else if (ms == 0) {
+                               /* An interval expired. */
+                               bridge_channel_handle_interval(bridge_channel);
                        }
                }
                bridge_channel->activity = AST_BRIDGE_CHANNEL_THREAD_IDLE;