[asterisk-commits] mmichelson: branch group/CCSS_Monitor_Restructure r244492 - /team/group/CCSS_...

[SVN commits to the Asterisk project]

Author: mmichelson
Date: Tue Feb  2 17:29:53 2010
New Revision: 244492

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=244492
Log:
Fix a race condition in generic device monitoring.

There was a race condition that could happen where a device
state event would fire between when we destroyed our generic
monitor and when we unsubscribed from device state. The problem
was that the device state event would result in incrementing and
decrementing an invalid reference to the freed monitor.

I have addressed this by having the device state callback not
have a pointer to the monitor, and instead has just the name
of the monitor. This string is not freed until after the
event is unsubscribed, thus averting the race condition.


Modified:
    team/group/CCSS_Monitor_Restructure/main/ccss.c

Modified: team/group/CCSS_Monitor_Restructure/main/ccss.c
URL: http://svnview.digium.com/svn/asterisk/team/group/CCSS_Monitor_Restructure/main/ccss.c?view=diff&rev=244492&r1=244491&r2=244492
==============================================================================
--- team/group/CCSS_Monitor_Restructure/main/ccss.c (original)
+++ team/group/CCSS_Monitor_Restructure/main/ccss.c Tue Feb  2 17:29:53 2010
@@ -938,11 +938,11 @@
 	struct generic_monitor_instance_list *generic_list = obj;
 	struct generic_monitor_instance *generic_instance;
 
+	generic_list->sub = ast_event_unsubscribe(generic_list->sub);
 	while ((generic_instance = AST_LIST_REMOVE_HEAD(&generic_list->list, next))) {
 		ast_free(generic_instance);
 	}
 	ast_free((char *)generic_list->device_name);
-	generic_list->sub = ast_event_unsubscribe(generic_list->sub);
 }
 
 static void generic_monitor_devstate_cb(const struct ast_event *event, void *userdata);
@@ -960,7 +960,7 @@
 	}
 
 	if (!(generic_list->sub = ast_event_subscribe(AST_EVENT_DEVICE_STATE, generic_monitor_devstate_cb,
-				"Requesting CC", monitor, AST_EVENT_IE_DEVICE, AST_EVENT_IE_PLTYPE_STR, 
+				"Requesting CC", (void *)generic_list->device_name, AST_EVENT_IE_DEVICE, AST_EVENT_IE_PLTYPE_STR, 
 				monitor->interface->name, AST_EVENT_IE_END))) {
 		cc_unref(generic_list, "Failed to subscribe to device state");
 		return NULL;
@@ -971,16 +971,15 @@
 }
 
 struct generic_tp_cb_data {
-	struct ast_cc_monitor *monitor;
+	const char *device_name;
 	enum ast_device_state new_state;
 };
 
 static int generic_monitor_devstate_tp_cb(void *data)
 {
 	struct generic_tp_cb_data *gtcd = data;
-	struct ast_cc_monitor *monitor = gtcd->monitor;
 	enum ast_device_state new_state = gtcd->new_state;
-	const char *monitor_name = monitor->interface->name;
+	const char *monitor_name = gtcd->device_name;
 	struct generic_monitor_instance_list *generic_list;
 	struct generic_monitor_instance *generic_instance;
 
@@ -989,14 +988,12 @@
 		 * time between subscribing to its device state and the time this executes.
 		 * Not really a big deal.
 		 */
-		cc_unref(gtcd->monitor, "Can't find generic list. Unref monitor in generic_monitor_devstate_tp_cb");
 		ast_free(gtcd);
 		return 0;
 	}
 
 	if (generic_list->current_state == new_state) {
 		/* The device state hasn't actually changed, so we don't really care */
-		cc_unref(gtcd->monitor, "State hasn't changed. Done with monitor in generic_monitor_devstate_tp_cb");
 		ast_free(gtcd);
 		cc_unref(generic_list, "Kill reference of generic list in devstate taskprocessor callback");
 		return 0;
@@ -1013,7 +1010,6 @@
 		}	
 	}
 	cc_unref(generic_list, "Kill reference of generic list in devstate taskprocessor callback");
-	cc_unref(gtcd->monitor, "Done with monitor ref in generic_monitor_devstate_tp_cb");
 	ast_free(gtcd);
 	return 0;
 }
@@ -1025,18 +1021,16 @@
 	 * so that all monitor operations can be serialized. Locks?! We don't need
 	 * no steenkin' locks!
 	 */
-	struct ast_cc_monitor *monitor = userdata;
 	struct generic_tp_cb_data *gtcd = ast_calloc(1, sizeof(*gtcd));
 
 	if (!gtcd) {
 		return;
 	}
 
-	gtcd->monitor = cc_ref(monitor, "Getting monitor reference for taskprocessor callback");
+	gtcd->device_name = userdata;
 	gtcd->new_state = ast_event_get_ie_uint(event, AST_EVENT_IE_STATE);
 
 	if (ast_taskprocessor_push(cc_core_taskprocessor, generic_monitor_devstate_tp_cb, gtcd)) {
-		cc_unref(gtcd->monitor, "Couldn't push taskprocessor callback. unref monitor");
 		ast_free(gtcd);
 	}
 }