[asterisk-commits] schmidts: branch schmidts/unleash-the-beast r340107 - /team/schmidts/unleash-...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Mon Oct 10 03:42:08 CDT 2011


Author: schmidts
Date: Mon Oct 10 03:42:04 2011
New Revision: 340107

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=340107
Log:
Fix a wrong refcounter problem, a not used destroy function and also a deadlock issue


Modified:
    team/schmidts/unleash-the-beast/main/pbx.c

Modified: team/schmidts/unleash-the-beast/main/pbx.c
URL: http://svnview.digium.com/svn/asterisk/team/schmidts/unleash-the-beast/main/pbx.c?view=diff&rev=340107&r1=340106&r2=340107
==============================================================================
--- team/schmidts/unleash-the-beast/main/pbx.c (original)
+++ team/schmidts/unleash-the-beast/main/pbx.c Mon Oct 10 03:42:04 2011
@@ -956,17 +956,20 @@
 static const int HASH_EXTENHINT_SIZE = 563;
 #endif
 
-/*! \brief Container for hint devices
-*/
+/*! \brief Container for hint devices */
 static struct ao2_container *hintdevices;
 
-/*! \brief Structure for dial plan hint devices
-
-  \note hintdevice is one device pointing to a hint.
-*/
+/*! 
+ * \brief Structure for dial plan hint devices
+ * \note hintdevice is one device pointing to a hint.
+ */
 struct ast_hintdevice {
-
+	/*!
+	 * \brief Hint this hintdevice belongs to.
+	 * \note Holds a reference to the hint object.
+	 */
 	struct ast_hint *hint;
+	/*! Name of the hint device. */
 	char hintdevice[1];
 };
 
@@ -982,7 +985,7 @@
 }
 /*!
  * \note Devices on hints are not unique so no CMP_STOP is returned
- * Dont use ao2_find against hintdevices container cause there allways
+ * Dont use ao2_find against hintdevices container cause there always
  * could be more than one result.
  */
 static int hintdevice_cmp_multiple(void *obj, void *arg, int flags)
@@ -1006,11 +1009,29 @@
 
 static int remove_hintdevice(struct ast_hint *hint)
 {
-
 	/* iterate through all devices and remove the devices which are linked to this hint */
-	ao2_t_callback(hintdevices, OBJ_NODATA | OBJ_MULTIPLE | OBJ_UNLINK, hintdevice_remove_cb, hint ,
+	ao2_t_callback(hintdevices, OBJ_NODATA | OBJ_MULTIPLE | OBJ_UNLINK, 
+		hintdevice_remove_cb, hint,
 		"callback to remove all devices which are linked to a hint");
 	return 0;
+}
+
+/*!
+ * \internal
+ * \brief Destroy the given hintdevice object.
+ *
+ * \param obj Hint device to destroy.
+ *
+ * \return Nothing
+ */
+static void hintdevice_destroy(void *obj)
+{
+	struct ast_hintdevice *doomed = obj;
+
+	if (doomed->hint) {
+		ao2_ref(doomed->hint, -1);
+		doomed->hint = NULL;
+	}
 }
 
 /*! \brief add hintdevice structure and link it into the container.
@@ -1018,10 +1039,15 @@
 static int add_hintdevice(struct ast_hint *hint, const char *devicelist)
 {
 	struct ast_str *str;
-	char *cur=NULL,*parse;
+	char *parse;
+	char *cur;
 	struct ast_hintdevice *device;
-	int devicelength = 0;
-
+	int devicelength;
+
+	if (!hint || !devicelist) {
+		/* Trying to add garbage? Don't bother. */
+		return 0;
+	}
 	if (!(str = ast_str_thread_get(&hintdevice_data, 16))) {
 		return -1;
 	}
@@ -1030,10 +1056,13 @@
 
 	while ((cur = strsep(&parse, "&"))) {
 		devicelength = strlen(cur);
-		if (!(device = ao2_t_alloc(sizeof(*device) + devicelength, NULL, "allocating a hintdevice structure"))) {
+		device = ao2_t_alloc(sizeof(*device) + devicelength, hintdevice_destroy,
+			"allocating a hintdevice structure");
+		if (!device) {
 			return -1;
 		}
 		strcpy(device->hintdevice, cur);
+		ao2_ref(hint, +1);
 		device->hint = hint;
 		ao2_t_link(hintdevices, device, "Linking device into hintdevice container.");
 		ao2_t_ref(device, -1, "hintdevice is linked so we can unref");
@@ -4362,7 +4391,8 @@
 {
 	struct ast_hint *hint;
 	struct ast_str *hint_app;
-	struct ast_hintdevice *device, *cmpdevice;
+	struct ast_hintdevice *device;
+	struct ast_hintdevice *cmpdevice;
 	struct statechange *sc = datap;
 	struct ao2_iterator *dev_iter;
 	struct ao2_iterator cb_iter;
@@ -4370,9 +4400,20 @@
 	char exten_name[AST_MAX_EXTENSION];
 
 	if (ao2_container_count(hintdevices) == 0) {
+		/* There are no hints monitoring devices. */
+		ast_free(sc);
 		return 0;
 	}
-	if (!(cmpdevice = ast_malloc(sizeof(*cmpdevice) + strlen(sc->dev)))) {
+
+	hint_app = ast_str_create(1024);
+	if (!hint_app) {
+		ast_free(sc);
+		return -1;
+	}
+
+	cmpdevice = alloca(sizeof(*cmpdevice) + strlen(sc->dev));
+	if (!cmpdevice) {
+		ast_free(sc);
 		return -1;
 	}
 	strcpy(cmpdevice->hintdevice, sc->dev);
@@ -4386,29 +4427,24 @@
 	
 	if (!dev_iter) {
 		ast_mutex_unlock(&context_merge_lock);
+		ast_free(hint_app);
 		ast_free(sc);
 		return -1;
 	}
-	hint_app = ast_str_create(1024);
-	if (!hint_app) {
-		ast_free(sc);
-		return -1;
-	}
-	for (; (device = ao2_iterator_next(dev_iter)); ao2_ref(device, -1)) {
+	for (; (device = ao2_iterator_next(dev_iter)); ao2_t_ref(device, -1, "Next device")) {
 		struct ast_state_cb *state_cb;
 		int state;
 		if (!device->hint) {
+			/* Should never happen. */
 			continue;
 		}
 
 		hint = device->hint;
-		ao2_lock(hints);
 		ao2_lock(hint);
 
 		if (!hint->exten) {
 			/* The extension has already been destroyed */
 			ao2_unlock(hint);
-			ao2_unlock(hints);
 			continue;
 		}
 
@@ -4423,7 +4459,6 @@
 			sizeof(exten_name));
 		ast_str_set(&hint_app, 0, "%s", ast_get_extension_app(hint->exten));
 		ao2_unlock(hint);
-		ao2_unlock(hints);
 
 		/*
 		 * Get device state for this hint.
@@ -4465,18 +4500,11 @@
 		}
 		ao2_iterator_destroy(&cb_iter);
 	}
+	ast_mutex_unlock(&context_merge_lock);
+
 	ao2_iterator_destroy(dev_iter);
-	ast_mutex_unlock(&context_merge_lock);
-
-	if (hint_app) {
-		ast_free(hint_app);
-		hint_app = NULL;
-	}
+	ast_free(hint_app);
 	ast_free(sc);
-	if (cmpdevice) {
-		ast_free(cmpdevice);
-		cmpdevice = NULL;
-	}
 	return 0;
 }
 
@@ -4567,7 +4595,7 @@
 	return id;
 }
 
-/*! \brief Remove a watcher from the callback list */
+/*! \brief Find Hint by callback id */
 static int find_hint_by_cb_id(void *obj, void *arg, int flags)
 {
 	struct ast_state_cb *state_cb;
@@ -4646,7 +4674,6 @@
 			context_name = ast_get_context_name(ast_get_extension_context(hint->exten));
 			exten_name = ast_get_extension_name(hint->exten);
 			hint->exten = NULL;
-			remove_hintdevice(hint);
 		} else {
 			/* The extension has already been destroyed */
 			context_name = hint->context_name;
@@ -4677,6 +4704,7 @@
 	if (!hint) {
 		return -1;
 	}
+	remove_hintdevice(hint);
 
 	/*
 	 * The extension is being destroyed so we must save some
@@ -4727,7 +4755,6 @@
 
 	/* Prevent multiple add hints from adding the same hint at the same time. */
 	ao2_lock(hints);
-	ao2_lock(hintdevices);
 
 	/* Search if hint exists, do nothing */
 	hint_found = ao2_find(hints, e, 0);
@@ -4751,7 +4778,6 @@
 	}
 
 	ao2_unlock(hints);
-	ao2_unlock(hintdevices);
 	ao2_ref(hint_new, -1);
 
 	return 0;
@@ -4777,18 +4803,18 @@
 		ao2_unlock(hints);
 		return -1;
 	}
+	remove_hintdevice(hint);
 
 	/* Update the hint and put it back in the hints container. */
 	ao2_lock(hint);
 	hint->exten = ne;
-	remove_hintdevice(hint);
+	ao2_unlock(hint);
+	ao2_link(hints, hint);
 	if (add_hintdevice(hint, ast_get_extension_app(ne))) {
 		ast_log(LOG_WARNING, "Could not add devices for hint: %s@%s.\n",
 			ast_get_extension_name(ne),
 			ast_get_context_name(ast_get_extension_context(ne)));
 	}
-	ao2_unlock(hint);
-	ao2_link(hints, hint);
 
 	ao2_unlock(hints);
 	ao2_ref(hint, -1);
@@ -10617,5 +10643,5 @@
 	hintdevices = ao2_container_alloc(HASH_EXTENHINT_SIZE, hintdevice_hash_cb, hintdevice_cmp_multiple);
 	statecbs = ao2_container_alloc(HASH_EXTENHINT_SIZE, NULL, statecbs_cmp);
 
-	return (hints && statecbs) ? 0 : -1;
-}
+	return (hints && hintdevices && statecbs) ? 0 : -1;
+}




More information about the asterisk-commits mailing list