[asterisk-commits] russell: branch rmudgett/misdn_facility r189346 - /team/rmudgett/misdn_facili...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Mon Apr 20 11:04:36 CDT 2009
Author: russell
Date: Mon Apr 20 11:04:32 2009
New Revision: 189346
URL: http://svn.digium.com/svn-view/asterisk?view=rev&rev=189346
Log:
Re-work some channel handling to avoid the potential for crashes.
Review: http://reviewboard.digium.com/r/226/
Modified:
team/rmudgett/misdn_facility/channels/chan_misdn.c
Modified: team/rmudgett/misdn_facility/channels/chan_misdn.c
URL: http://svn.digium.com/svn-view/asterisk/team/rmudgett/misdn_facility/channels/chan_misdn.c?view=diff&rev=189346&r1=189345&r2=189346
==============================================================================
--- team/rmudgett/misdn_facility/channels/chan_misdn.c (original)
+++ team/rmudgett/misdn_facility/channels/chan_misdn.c Mon Apr 20 11:04:32 2009
@@ -132,8 +132,6 @@
/* BEGIN: chan_misdn.h */
#if defined(AST_MISDN_ENHANCEMENTS)
-#define MISDN_PEER_LINKS_MAX 64
-#define MISDN_PEER_AGE_MAX 20 /* seconds */
/*
* This timeout duration is to clean up any call completion records that
@@ -143,18 +141,19 @@
#define MISDN_CC_REQUEST_WAIT_MAX 5 /* seconds */
-struct misdn_peer_link {
- /*! \brief Asterisk channel that is a potential peer (NULL if not allocated) */
- struct ast_channel *peer;
-
- /*! \brief Time the record was allocated. */
- time_t time_created;
+/*!
+ * \brief Caller that initialized call completion services
+ *
+ * This data is the payload for a datastore that is put on the channel that
+ * initializes call completion services. This datastore is set to be inherited
+ * by the outbound mISDN channel. When one of these channels hangs up, the
+ * channel pointer will be set to NULL. That way, we can ensure that we do not
+ * touch this channel after it gets destroyed.
+ */
+struct misdn_cc_caller {
+ /*! \brief The channel that initialized call completion services */
+ struct ast_channel *chan;
};
-
-/*! \brief Peer link guardian */
-AST_MUTEX_DEFINE_STATIC(misdn_peer_link_lock);
-/*! \brief mISDN peer link database */
-static struct misdn_peer_link misdn_peers[MISDN_PEER_LINKS_MAX];
struct misdn_cc_notify {
/*! \brief Dialplan: Notify extension priority */
@@ -491,8 +490,10 @@
struct misdn_bchannel *bc;
#if defined(AST_MISDN_ENHANCEMENTS)
- /*! \brief Asterisk channel that initiated this call (may be NULL) */
- struct ast_channel *peer;
+ /*!
+ * \brief Peer channel for which call completion was initialized.
+ */
+ struct misdn_cc_caller *peer;
/*! \brief Associated call completion record ID (-1 if not associated) */
long record_id;
@@ -762,69 +763,127 @@
#if defined(AST_MISDN_ENHANCEMENTS)
/*!
* \internal
- * \brief Get the requested peer link.
- *
- * \param peer_id Peer link id to obtain
- *
- * \retval peer_channel on success.
- * \retval NULL on error.
+ * \brief Destroy the misdn_cc_ds_info datastore payload
+ *
+ * \param[in] data the datastore payload, a reference to an misdn_cc_caller
+ *
+ * \details
+ * Since the payload is a reference to an astobj2 object, we just decrement its
+ * reference count. Before doing so, we NULL out the channel pointer inside of
+ * the misdn_cc_caller instance. This function will be called in one of two
+ * cases. In both cases, we no longer need the channel pointer:
+ *
+ * - The original channel that initialized call completion services, the same
+ * channel that is stored here, has been destroyed early. This could happen
+ * if it transferred the mISDN channel, for example.
+ *
+ * - The mISDN channel that had this datastore inherited on to it is now being
+ * destroyed. If this is the case, then the call completion events have
+ * already occurred and the appropriate channel variables have already been
+ * set on the original channel that requested call completion services.
+ *
+ * \return Nothing
*/
-static struct ast_channel *misdn_peer_link_get(unsigned peer_id)
-{
- struct ast_channel *peer;
-
- if (peer_id < ARRAY_LEN(misdn_peers)) {
- ast_mutex_lock(&misdn_peer_link_lock);
- peer = misdn_peers[peer_id].peer;
- ast_mutex_unlock(&misdn_peer_link_lock);
- } else {
- peer = NULL;
- }
-
- return peer;
+static void misdn_cc_ds_destroy(void *data)
+{
+ struct misdn_cc_caller *cc_caller = data;
+
+ ao2_lock(cc_caller);
+ cc_caller->chan = NULL;
+ ao2_unlock(cc_caller);
+
+ ao2_ref(cc_caller, -1);
}
#endif /* defined(AST_MISDN_ENHANCEMENTS) */
#if defined(AST_MISDN_ENHANCEMENTS)
/*!
* \internal
- * \brief Allocate a new peer link association id.
- *
- * \param chan Asterisk channel that is a potential peer.
- *
- * \retval peer_link_id on success.
- * \retval -1 on error.
+ * \brief Duplicate the misdn_cc_ds_info datastore payload
+ *
+ * \param[in] data the datastore payload, a reference to an misdn_cc_caller
+ *
+ * \details
+ * All we need to do is bump the reference count and return the same instance.
+ *
+ * \return A reference to an instance of a misdn_cc_caller
*/
-static int misdn_peer_link_new(struct ast_channel *chan)
-{
- int index;
- int peer_id;
- time_t now;
-
- ast_mutex_lock(&misdn_peer_link_lock);
-
- /* Remove all old entries */
- now = time(NULL);
- for (index = 0; index < ARRAY_LEN(misdn_peers); ++index) {
- if (misdn_peers[index].peer
- && MISDN_PEER_AGE_MAX < now - misdn_peers[index].time_created) {
- misdn_peers[index].peer = NULL;
- }
- }
-
- /* Allocate a peer_id */
- peer_id = -1;
- for (index = 0; index < ARRAY_LEN(misdn_peers); ++index) {
- if (!misdn_peers[index].peer) {
- misdn_peers[index].peer = chan;
- misdn_peers[index].time_created = time(NULL);
- peer_id = index;
- break;
- }
- }
- ast_mutex_unlock(&misdn_peer_link_lock);
-
- return peer_id;
+static void *misdn_cc_ds_duplicate(void *data)
+{
+ struct misdn_cc_caller *cc_caller = data;
+
+ ao2_ref(cc_caller, +1);
+
+ return cc_caller;
+}
+#endif /* defined(AST_MISDN_ENHANCEMENTS) */
+
+#if defined(AST_MISDN_ENHANCEMENTS)
+static const struct ast_datastore_info misdn_cc_ds_info = {
+ .type = "misdn_cc",
+ .destroy = misdn_cc_ds_destroy,
+ .duplicate = misdn_cc_ds_duplicate,
+};
+#endif /* defined(AST_MISDN_ENHANCEMENTS) */
+
+#if defined(AST_MISDN_ENHANCEMENTS)
+/*!
+ * \internal
+ * \brief Set a channel var on the peer channel for call completion services
+ *
+ * \param[in] peer The peer that initialized call completion services
+ * \param[in] var The variable name to set
+ * \param[in] value The variable value to set
+ *
+ * This function may be called from outside of the channel thread. It handles
+ * the fact that the peer channel may be hung up and destroyed at any time.
+ *
+ * \return nothing
+ */
+static void misdn_cc_set_peer_var(struct misdn_cc_caller *peer, const char *var,
+ const char *value)
+{
+ ao2_lock(peer);
+
+ /* XXX This nastiness can go away once ast_channel is ref counted! */
+ while (peer->chan && ast_channel_trylock(peer->chan)) {
+ ao2_unlock(peer);
+ sched_yield();
+ ao2_lock(peer);
+ }
+
+ if (peer->chan) {
+ pbx_builtin_setvar_helper(peer->chan, var, value);
+ ast_channel_unlock(peer->chan);
+ }
+
+ ao2_unlock(peer);
+}
+#endif /* defined(AST_MISDN_ENHANCEMENTS) */
+
+#if defined(AST_MISDN_ENHANCEMENTS)
+/*!
+ * \internal
+ * \brief Get a reference to the CC caller if it exists
+ */
+static struct misdn_cc_caller *misdn_cc_caller_get(struct ast_channel *chan)
+{
+ struct ast_datastore *datastore;
+ struct misdn_cc_caller *cc_caller;
+
+ ast_channel_lock(chan);
+
+ if (!(datastore = ast_channel_datastore_find(chan, &misdn_cc_ds_info, NULL))) {
+ ast_channel_unlock(chan);
+ return NULL;
+ }
+
+ ao2_ref(datastore->data, +1);
+ cc_caller = datastore->data;
+
+ ast_channel_unlock(chan);
+
+ return cc_caller;
}
#endif /* defined(AST_MISDN_ENHANCEMENTS) */
@@ -1125,12 +1184,10 @@
{
struct misdn_cc_record *current;
- AST_LIST_TRAVERSE_SAFE_BEGIN(&misdn_cc_records_db, current, list) {
+ while ((current = AST_LIST_REMOVE_HEAD(&misdn_cc_records_db, list))) {
/* Do a misdn_cc_delete(current) inline */
- AST_LIST_REMOVE_CURRENT(list);
ast_free(current);
}
- AST_LIST_TRAVERSE_SAFE_END;
}
#endif /* defined(AST_MISDN_ENHANCEMENTS) */
@@ -6183,9 +6240,6 @@
struct chan_list *ch;
struct misdn_bchannel *newbc;
char *dest_cp;
-#if defined(AST_MISDN_ENHANCEMENTS)
- const char *chan_value;
-#endif /* defined(AST_MISDN_ENHANCEMENTS) */
AST_DECLARE_APP_ARGS(args,
AST_APP_ARG(intf); /* The interface token is discarded. */
@@ -6224,22 +6278,10 @@
port = newbc->port;
#if defined(AST_MISDN_ENHANCEMENTS)
- /*
- * Setup struct chan_list.peer if MISDN_CC_PEER_ID has a value.
- * NOTE: The variable is not available until after misdn_request()
- * has been called.
- */
- ast_channel_lock(ast);
- chan_value = pbx_builtin_getvar_helper(ast, MISDN_CC_PEER_ID);
- if (chan_value) {
- int peer_id;
-
- peer_id = atoi(chan_value);
- ch->peer = misdn_peer_link_get(peer_id);
- chan_misdn_log(3, port, " --> Found %s:%s, peer:%s\n",
- MISDN_CC_PEER_ID, chan_value, ch->peer ? "available" : "NULL");
- }
- ast_channel_unlock(ast);
+ if ((ch->peer = misdn_cc_caller_get(ast))) {
+ chan_misdn_log(3, port, " --> Found CC caller data, peer:%s\n",
+ ch->peer->chan ? "available" : "NULL");
+ }
if (ch->record_id != -1) {
struct misdn_cc_record *cc_record;
@@ -6880,6 +6922,13 @@
}
p->state = MISDN_CLEANING;
+
+#if defined(AST_MISDN_ENHANCEMENTS)
+ if (p->peer) {
+ ao2_ref(p->peer, -1);
+ p->peer = NULL;
+ }
+#endif /* AST_MISDN_ENHANCEMENTS */
chan_misdn_log(3, bc->port, " --> Channel: %s hanguped new state:%s\n", ast->name, misdn_get_ch_state(p));
@@ -8702,7 +8751,7 @@
case EVENT_RELEASE_COMPLETE:
/* Possible call failure as a result of Fac_CCBSCall/Fac_CCBS_T_Call */
if (ch && ch->peer) {
- pbx_builtin_setvar_helper(ch->peer, MISDN_ERROR_MSG, diagnostic_msg);
+ misdn_cc_set_peer_var(ch->peer, MISDN_ERROR_MSG, diagnostic_msg);
}
break;
default:
@@ -8725,7 +8774,7 @@
case EVENT_RELEASE_COMPLETE:
/* Possible call failure as a result of Fac_CCBSCall/Fac_CCBS_T_Call */
if (ch && ch->peer) {
- pbx_builtin_setvar_helper(ch->peer, MISDN_ERROR_MSG, diagnostic_msg);
+ misdn_cc_set_peer_var(ch->peer, MISDN_ERROR_MSG, diagnostic_msg);
}
break;
default:
@@ -8880,7 +8929,8 @@
} else {
buf[0] = 0;
}
- pbx_builtin_setvar_helper(ch->peer, MISDN_CC_RECORD_ID, buf);
+
+ misdn_cc_set_peer_var(ch->peer, MISDN_CC_RECORD_ID, buf);
}
break;
default:
@@ -9093,7 +9143,8 @@
} else {
buf[0] = 0;
}
- pbx_builtin_setvar_helper(ch->peer, MISDN_CC_RECORD_ID, buf);
+
+ misdn_cc_set_peer_var(ch->peer, MISDN_CC_RECORD_ID, buf);
}
break;
default:
@@ -9786,12 +9837,9 @@
AST_LIST_UNLOCK(&misdn_cc_records_db);
ch->record_id = -1;
if (ch->peer) {
- pbx_builtin_setvar_helper(ch->peer, MISDN_CC_RECORD_ID, "");
-
- /*
- * The peer pointer is no longer needed and keeping it around
- * could be dangerous.
- */
+ misdn_cc_set_peer_var(ch->peer, MISDN_CC_RECORD_ID, "");
+
+ ao2_ref(ch->peer, -1);
ch->peer = NULL;
}
}
@@ -10610,6 +10658,28 @@
#endif /* defined(AST_MISDN_ENHANCEMENTS) */
#if defined(AST_MISDN_ENHANCEMENTS)
+static void misdn_cc_caller_destroy(void *obj)
+{
+ /* oh snap! */
+}
+#endif /* defined(AST_MISDN_ENHANCEMENTS) */
+
+#if defined(AST_MISDN_ENHANCEMENTS)
+static struct misdn_cc_caller *misdn_cc_caller_alloc(struct ast_channel *chan)
+{
+ struct misdn_cc_caller *cc_caller;
+
+ if (!(cc_caller = ao2_alloc(sizeof(*cc_caller), misdn_cc_caller_destroy))) {
+ return NULL;
+ }
+
+ cc_caller->chan = chan;
+
+ return cc_caller;
+}
+#endif /* defined(AST_MISDN_ENHANCEMENTS) */
+
+#if defined(AST_MISDN_ENHANCEMENTS)
/*!
* \internal
* \brief misdn_command(cc-initialize) subcommand handler
@@ -10622,17 +10692,29 @@
*/
static int misdn_command_cc_initialize(struct ast_channel *chan, struct misdn_command_args *subcommand)
{
- int peer_id;
- char buf[32];
-
- peer_id = misdn_peer_link_new(chan);
- if (peer_id < 0) {
- ast_log(LOG_WARNING, "Could not allocate peer link record!\n");
+ struct misdn_cc_caller *cc_caller;
+ struct ast_datastore *datastore;
+
+ if (!(cc_caller = misdn_cc_caller_alloc(chan))) {
return -1;
}
- snprintf(buf, sizeof(buf), "%d", peer_id);
- pbx_builtin_setvar_helper(chan, "_" MISDN_CC_PEER_ID, buf);
+ if (!(datastore = ast_datastore_alloc(&misdn_cc_ds_info, NULL))) {
+ ao2_ref(cc_caller, -1);
+ return -1;
+ }
+
+ ast_channel_lock(chan);
+
+ /* Inherit reference */
+ datastore->data = cc_caller;
+ cc_caller = NULL;
+
+ datastore->inheritance = DATASTORE_INHERIT_FOREVER;
+
+ ast_channel_datastore_add(chan, datastore);
+
+ ast_channel_unlock(chan);
return 0;
}
More information about the asterisk-commits
mailing list