[asterisk-commits] twilson: branch twilson/calendaring r162620 - in /team/twilson/calendaring: m...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Dec 10 09:25:03 CST 2008


Author: twilson
Date: Wed Dec 10 09:25:03 2008
New Revision: 162620

URL: http://svn.digium.com/view/asterisk?view=rev&rev=162620
Log:
Try to take care of a sched race condition, rework in more ao2_callbacks

Modified:
    team/twilson/calendaring/main/calendar.c
    team/twilson/calendaring/res/res_caldav.c
    team/twilson/calendaring/res/res_exchangecal.c
    team/twilson/calendaring/res/res_icalendar.c

Modified: team/twilson/calendaring/main/calendar.c
URL: http://svn.digium.com/view/asterisk/team/twilson/calendaring/main/calendar.c?view=diff&rev=162620&r1=162619&r2=162620
==============================================================================
--- team/twilson/calendaring/main/calendar.c (original)
+++ team/twilson/calendaring/main/calendar.c Wed Dec 10 09:25:03 2008
@@ -275,10 +275,14 @@
 {
 	struct ast_calendar_event *event = obj;
 
+	ao2_lock(event);
 	ast_debug(3, "Destroying event for calendar '%s'\n", event->owner->name);
 	ast_string_field_free_memory(event);
-	if (event->notify_sched >= 0) {
-		AST_SCHED_DEL(sched, event->notify_sched);
+
+	while (event->notify_sched > -1 && ast_sched_del(sched, event->notify_sched)) {
+		ao2_unlock(event);
+		usleep(1);
+		ao2_lock(event);
 	}
 
 	/* If an event is being deleted and we've fired an event changing the status at the beginning,
@@ -291,12 +295,19 @@
 		}
 	}
 
-	if (event->bs_start_sched >= 0) {
-		AST_SCHED_DEL(sched, event->bs_start_sched);
-	}
-	if (event->bs_end_sched >= 0) {
-		AST_SCHED_DEL(sched, event->bs_end_sched);
-	}
+	while (event->bs_start_sched >= 0 && ast_sched_del(sched, event->bs_start_sched)) {
+		ao2_unlock(event);
+		usleep(1);
+		ao2_lock(event);
+	}
+
+	while (event->bs_end_sched > -1 && ast_sched_del(sched, event->bs_end_sched)) {
+		ao2_unlock(event);
+		usleep(1);
+		ao2_lock(event);
+	}
+
+	ao2_unlock(event);
 
 	return;
 }
@@ -361,10 +372,24 @@
 	return event;
 }
 
+/*! \brief Generate 32 byte random string (stolen from chan_sip.c)*/
+static char *generate_random_string(char *buf, size_t size)
+{
+	long val[4];
+	int x;
+
+	for (x = 0; x < 4; x++) {
+		val[x] = ast_random();
+	}
+	snprintf(buf, size, "%08lx%08lx%08lx%08lx", val[0], val[1], val[2], val[3]);
+
+	return buf;
+}
+
 static int calendar_event_notify(const void *data)
 {
 	struct ast_calendar_event *event = (void *)data;
-	char tech[256], dest[256], *tmp;
+	char tech[256], dest[256], buf[8], *tmp;
 	struct ast_dial *dial = NULL;
 	struct ast_channel *chan = NULL;
 	struct ast_str *apptext = NULL;
@@ -376,6 +401,10 @@
 		ast_log(LOG_ERROR, "Extremely low-cal...in fact cal is NULL!\n");
 		goto notify_cleanup;
 	}
+
+	ao2_lock(event);
+
+	event->notify_sched = -1;
 
 	ast_copy_string(tech, event->owner->notify_channel, sizeof(tech));
 
@@ -399,7 +428,8 @@
 	}
 
 	ast_dial_set_global_timeout(dial, event->owner->notify_waittime);
-	if (!(chan = ast_channel_alloc(1, AST_STATE_DOWN, 0, 0, 0, 0, 0, 0, "Calendar/%s-%d", event->owner->name, event->notify_sched))) {
+	generate_random_string(buf, sizeof(buf));
+	if (!(chan = ast_channel_alloc(1, AST_STATE_DOWN, 0, 0, 0, 0, 0, 0, "Calendar/%s-%s", event->owner->name, buf))) {
 		ast_log(LOG_ERROR, "Could not allocate notification channel\n");
 		goto notify_cleanup;
 	}
@@ -437,7 +467,7 @@
 	res = 0;
 
 notify_cleanup:
-	event->notify_sched = -1;
+	ao2_unlock(event);
 	if (res == -1 && dial) {
 		ast_dial_destroy(dial);
 	}
@@ -455,6 +485,22 @@
 {
 	struct ast_calendar_event *event = (struct ast_calendar_event *)data;
 	struct timeval now = ast_tvnow();
+	int is_end_event;
+
+	if (!event) {
+		ast_log(LOG_WARNING, "Event was NULL!\n");
+		return 0;
+	}
+
+	ao2_lock(event);
+
+	is_end_event = event->end <= now.tv_sec;
+
+	if (is_end_event) {
+		event->bs_end_sched = -1;
+	} else {
+		event->bs_start_sched = -1;
+	}
 
 	/* We can have overlapping events, so ignore the event->busy_state and check busy state
 	 * based on all events in the calendar */
@@ -464,97 +510,127 @@
 		ast_devstate_changed(AST_DEVICE_BUSY, "Calendar/%s", event->owner->name);
 	}
 
-	if (event->end <= now.tv_sec) {
-		event->bs_end_sched = -1;
-	}
-
-	event->bs_start_sched = -1;
+	ao2_unlock(event);
 
 	return 0;
 }
 
-static int schedule_calendar_event(struct ast_calendar *cal, struct ast_calendar_event *event)
+static void copy_event_data(struct ast_calendar_event *dst, struct ast_calendar_event *src)
+{
+	ast_string_field_set(dst, summary, src->summary);
+	ast_string_field_set(dst, description, src->description);
+	ast_string_field_set(dst, organizer, src->organizer);
+	ast_string_field_set(dst, location, src->location);
+	ast_string_field_set(dst, uid, src->uid);
+	dst->owner = src->owner;
+	dst->start = src->start;
+	dst->end = src->end;
+	dst->alarm = src->alarm;
+	dst->busy_state = src->busy_state;
+}
+
+static int schedule_calendar_event(struct ast_calendar *cal, struct ast_calendar_event *old_event, struct ast_calendar_event *cmp_event)
 {
 	struct timeval now = ast_tvnow();
+	struct ast_calendar_event *event;
 	time_t alarm_notify_sched = 0, devstate_sched_start, devstate_sched_end;
-
-	if (cal->autoreminder) {
-		alarm_notify_sched = (event->start - (60 * cal->autoreminder) - now.tv_sec) * 1000;
-	} else if (event->alarm) {
-		alarm_notify_sched = (event->alarm - now.tv_sec) * 1000;
-	}
-
-	/* For now, send the notification if we missed it, but the meeting hasn't happened yet */
-	if (event->start >=  now.tv_sec) {
-		if (alarm_notify_sched <= 0) {
-			alarm_notify_sched = 1;
-		}
-		AST_SCHED_REPLACE(event->notify_sched, sched, alarm_notify_sched, calendar_event_notify, event);
+	int changed = 0;
+
+	event = cmp_event ? cmp_event : old_event;
+
+	ao2_lock(event);
+	if (!cmp_event || old_event->alarm != event->alarm) {
+		changed = 1;
+		if (cal->autoreminder) {
+			alarm_notify_sched = (event->start - (60 * cal->autoreminder) - now.tv_sec) * 1000;
+		} else if (event->alarm) {
+			alarm_notify_sched = (event->alarm - now.tv_sec) * 1000;
+		}
+
+		/* For now, send the notification if we missed it, but the meeting hasn't happened yet */
+		if (event->start >=  now.tv_sec) {
+			if (alarm_notify_sched <= 0) {
+				alarm_notify_sched = 1;
+			}
+			AST_SCHED_REPLACE(old_event->notify_sched, sched, alarm_notify_sched, calendar_event_notify, old_event);
+			ast_debug(3, "(1)Calendar alarm event notification scheduled to happen in %ld ms\n", alarm_notify_sched);
+		}
+	}
+
+	if (!cmp_event || old_event->start != event->start) {
+		changed = 1;
+		devstate_sched_start = (event->start - now.tv_sec) * 1000;
+
+		if (devstate_sched_start < 1) {
+			devstate_sched_start = 1;
+		}
+
+		AST_SCHED_REPLACE(old_event->bs_start_sched, sched, devstate_sched_start, calendar_devstate_change, old_event);
+		ast_debug(3, "(1)Calendar bs_start event notification scheduled to happen in %ld ms\n", devstate_sched_start);
+	}
+
+	if (!cmp_event || old_event->end != event->end) {
+		changed = 1;
+		devstate_sched_end = (event->end - now.tv_sec) * 1000;
+		AST_SCHED_REPLACE(old_event->bs_end_sched, sched, devstate_sched_end, calendar_devstate_change, old_event);
+		ast_debug(3, "(1)Calendar bs_start event notification scheduled to happen in %ld ms\n", devstate_sched_end);
+	}
+
+	if (changed) {
 		ast_cond_signal(&refresh_condition);
-		ast_debug(3, "(1)Calendar alarm event notification scheduled to happen in %ld ms\n", alarm_notify_sched);
-	}
-
-	devstate_sched_start = (event->start - now.tv_sec) * 1000;
-	devstate_sched_end = (event->end - now.tv_sec) * 1000;
-
-	if (devstate_sched_start < 1) {
-		devstate_sched_start = 1;
-	}
-
-	AST_SCHED_REPLACE(event->bs_start_sched, sched, devstate_sched_start, calendar_devstate_change, event);
-	AST_SCHED_REPLACE(event->bs_end_sched, sched, devstate_sched_end, calendar_devstate_change, event);
-	ast_cond_signal(&refresh_condition);
+	}
+
+	ao2_unlock(event);
 
 	return 0;
 }
 
+static int merge_events_cb(void *obj, void *arg, int flags)
+{
+	struct ast_calendar_event *old_event = obj, *new_event;
+	struct ao2_container *new_events = arg;
+
+	/* If we don't find the old_event in new_events, then we can safely delete the old_event */
+	if (!(new_event = find_event(new_events, old_event->uid))) {
+		/* XXX We may need to remove scheduler events here */
+		return CMP_MATCH;
+	}
+
+	/* We have events to merge.  If any data that will affect a scheduler event has changed,
+	 * then we need to replace the scheduler event */
+	schedule_calendar_event(old_event->owner, old_event, new_event);
+
+	/* Since we don't want to mess with cancelling sched events and adding new ones, just
+	 * copy the internals of the new_event to the old_event */
+	copy_event_data(old_event, new_event);
+
+	/* Now we can go ahead and unlink the new_event from new_events and unref it so that only completely
+	 * new events remain in the container */
+	ao2_unlink(new_events, new_event);
+	new_event = ast_calendar_unref_event(new_event);
+
+	return 0;
+}
+
+static int add_new_event_cb(void *obj, void *arg, int flags)
+{
+	struct ast_calendar_event *new_event = obj;
+	struct ao2_container *events = arg;
+
+	ao2_link(events, new_event);
+	schedule_calendar_event(new_event->owner, new_event, NULL);
+	return CMP_MATCH;
+}
+
 void ast_calendar_merge_events(struct ast_calendar *cal, struct ao2_container *new_events)
 {
-	struct ao2_iterator i;
-	struct ast_calendar_event *old_event, *new_event;
-
-	i = ao2_iterator_init(new_events, 0);
-	while ((new_event = ao2_iterator_next(&i))) {
-		int needs_rescheduled = 0;
-
-		/* Completely new eent */
-		if (!(old_event = find_event(cal->events, new_event->uid))) {
-			schedule_calendar_event(cal, new_event);
-			new_event = ast_calendar_unref_event(new_event);
-			continue;
-		}
-
-		/* Event exists in old events, and may need to be rescheduled */
-		if (!(old_event->start == new_event->start && old_event->alarm == new_event->alarm && old_event->end == new_event->end)) {
-			needs_rescheduled = 1;
-		}
-
-		if (needs_rescheduled) {
-			ao2_unlink(cal->events, old_event);
-			old_event = ast_calendar_unref_event(old_event);
-
-			schedule_calendar_event(cal, new_event);
-
-			/* if an event is moved forward in time, we need to make sure the busy state is updated */
-			if (!calendar_is_busy(new_event->owner)) {
-				ast_devstate_changed(AST_DEVICE_NOT_INUSE, "calendar/%s", new_event->owner->name);
-			}
-		}
-		new_event = ast_calendar_unref_event(new_event);
-	}
-
-	/* Delete stale events in calendar */
-	if (ao2_container_count(cal->events)) {
-		ast_calendar_clear_events(cal);
-	}
-
-	/* It doesn't feel right to just point cal->events to new_events */
-	i = ao2_iterator_init(new_events, 0);
-	while ((new_event = ao2_iterator_next(&i))) {
-		ao2_link(cal->events, new_event);
-		ao2_unlink(new_events, new_event);
-		new_event = ast_calendar_unref_event(new_event);
-	}
+	/* Loop through all events attached to the calendar.  If there is a matching new event
+	 * merge its data over and handle any schedule changes that need to be made.  Then remove
+	 * the new_event from new_events so that we are left with only new_events that we can add later. */
+	ao2_callback(cal->events, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, merge_events_cb, new_events);
+
+	/* Now, we should only have completely new events in new_events.  Loop through and add them */
+	ao2_callback(new_events, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, add_new_event_cb, cal->events);
 }
 
 static int load_config(void *data)
@@ -777,20 +853,6 @@
 	return i;
 }
 
-/*! \brief Generate 32 byte random string (stolen from chan_sip.c)*/
-static char *generate_random_string(char *buf, size_t size)
-{
-	long val[4];
-	int x;
-
-	for (x = 0; x < 4; x++) {
-		val[x] = ast_random();
-	}
-	snprintf(buf, size, "%08lx%08lx%08lx%08lx", val[0], val[1], val[2], val[3]);
-
-	return buf;
-}
-
 static void eventlist_destroy(void *data)
 {
 	struct eventlist *events = data;

Modified: team/twilson/calendaring/res/res_caldav.c
URL: http://svn.digium.com/view/asterisk/team/twilson/calendaring/res/res_caldav.c?view=diff&rev=162620&r1=162619&r2=162620
==============================================================================
--- team/twilson/calendaring/res/res_caldav.c (original)
+++ team/twilson/calendaring/res/res_caldav.c Wed Dec 10 09:25:03 2008
@@ -151,7 +151,7 @@
 
 	ret = ne_request_dispatch(req);
 
-	if (ret != NE_OK) {
+	if (ret != NE_OK || !response->used) {
 		ast_log(LOG_WARNING, "Unknown response to CalDAV calendar %s, request %s to %s: %s\n", pvt->owner->name, method, pvt->url, ne_get_error(pvt->session));
 		ast_free(response);
 		return NULL;

Modified: team/twilson/calendaring/res/res_exchangecal.c
URL: http://svn.digium.com/view/asterisk/team/twilson/calendaring/res/res_exchangecal.c?view=diff&rev=162620&r1=162619&r2=162620
==============================================================================
--- team/twilson/calendaring/res/res_exchangecal.c (original)
+++ team/twilson/calendaring/res/res_exchangecal.c Wed Dec 10 09:25:03 2008
@@ -402,7 +402,7 @@
 
 	ret = ne_request_dispatch(req);
 
-	if (ret != NE_OK) {
+	if (ret != NE_OK || !response->used) {
 		ast_log(LOG_WARNING, "Unknown response to CalDAV calendar %s, request %s to %s: %s\n", pvt->owner->name, method, pvt->url, ne_get_error(pvt->session));
 		ast_free(response);
 		return NULL;

Modified: team/twilson/calendaring/res/res_icalendar.c
URL: http://svn.digium.com/view/asterisk/team/twilson/calendaring/res/res_icalendar.c?view=diff&rev=162620&r1=162619&r2=162620
==============================================================================
--- team/twilson/calendaring/res/res_icalendar.c (original)
+++ team/twilson/calendaring/res/res_icalendar.c Wed Dec 10 09:25:03 2008
@@ -144,7 +144,7 @@
 	ne_add_response_body_reader(req, ne_accept_2xx, fetch_response_reader, &response);
 
 	ret = ne_request_dispatch(req);
-	if (ret != NE_OK) {
+	if (ret != NE_OK || !response->used) {
 		ast_log(LOG_WARNING, "Unable to retrieve iCalendar '%s' from '%s': %s\n", pvt->owner->name, pvt->url, ne_get_error(pvt->session));
 		ast_free(response);
 		return NULL;




More information about the asterisk-commits mailing list