Change the lock to a recursive e-mutex. (camel_object_hook_event):

2001-10-02    <NotZed@Ximian.com>

        * camel-object.c (camel_object_get_hooks): Change the lock to a
        recursive e-mutex.
        (camel_object_hook_event): Maintain list length of hook list.
        (camel_object_unhook_event): "
        (camel_object_unhook_event): If we are in an event, just mark the
        pair as removed, without removing it.
        (camel_object_trigger_event): Before running events, copy the
        list, and also ignore 'removed' events.  After running events, if
        we're all out of events, then free up any pending-removed events.
        (camel_object_free_hooks): Add some new assertions on the state of
        the hook structure.
        Removed the #error if threads not defined.  It _should_ actually
        work without threads.
        (camel_object_free_hooks): Free mutex when done.

svn path=/trunk/; revision=13347

2 files changed, 89 insertions, 136 deletions

diff --git a/camel/ChangeLog b/camel/ChangeLog
index eb27d0fe62..c6664538be 100644
--- a/camel/ChangeLog
+++ b/camel/ChangeLog
@@ -1,3 +1,20 @@
+2001-10-02    <NotZed@Ximian.com>
+
+	* camel-object.c (camel_object_get_hooks): Change the lock to a
+	recursive e-mutex.
+	(camel_object_hook_event): Maintain list length of hook list.
+	(camel_object_unhook_event): "
+	(camel_object_unhook_event): If we are in an event, just mark the
+	pair as removed, without removing it.
+	(camel_object_trigger_event): Before running events, copy the
+	list, and also ignore 'removed' events.  After running events, if
+	we're all out of events, then free up any pending-removed events.
+	(camel_object_free_hooks): Add some new assertions on the state of
+	the hook structure.
+	Removed the #error if threads not defined.  It _should_ actually
+	work without threads.
+	(camel_object_free_hooks): Free mutex when done.
+
 2001-10-01  Jeffrey Stedfast  <fejj@ximian.com>
 
 	* providers/imap/camel-imap-store.c (delete_folder): Remove any
diff --git a/camel/camel-object.c b/camel/camel-object.c
index fde35f3be1..85be0bcf61 100644
--- a/camel/camel-object.c
+++ b/camel/camel-object.c
@@ -30,12 +30,10 @@
 #include <string.h>
 #include "camel-object.h"
 
-/* FIXME: Make the code so this isn't necessary, its just the hook locking thats the problem */
-#ifndef ENABLE_THREADS
-#error "Threads must be enabled to compile this version of camel"
-#endif
-
+#ifdef ENABLE_THREADS
 #include <pthread.h>
+#include <e-util/e-msgport.h>
+#endif
 
 /* I just mashed the keyboard for these... */
 #define CAMEL_OBJECT_MAGIC_VALUE           0x77A344EF
@@ -72,11 +70,17 @@ CamelTypeInfo;
 
 /* A 'locked' hooklist, that is only allocated on demand */
 typedef struct _CamelHookList {
-	pthread_mutex_t lock;
+	EMutex *lock;
+
+	unsigned int depth:30;	/* recursive event depth */
+	unsigned int flags:2;	/* flags, see below */
 
+	unsigned int list_length;
 	struct _CamelHookPair *list;
 } CamelHookList;
 
+#define CAMEL_HOOK_PAIR_REMOVED (1<<0)
+
 /* a 'hook pair', actually a hook tuple, we just store all hooked events in the same list,
    and just comapre as we go, rather than storing separate lists for each hook type
 
@@ -87,6 +91,8 @@ typedef struct _CamelHookPair
 {
 	struct _CamelHookPair *next; /* next MUST be the first member */
 
+	unsigned int flags;	/* removed, etc */
+
 	const char *name;	/* points to the key field in the classes preplist, static memory */
 	CamelObjectEventHookFunc func;
 	void *data;
@@ -711,12 +717,17 @@ static void camel_object_free_hooks(CamelObject *o)
 	CamelHookPair *pair, *next;
 
 	if (o->hooks) {
+
+		g_assert(o->hooks->depth == 0);
+		g_assert((o->hooks->flags & CAMEL_HOOK_PAIR_REMOVED) == 0);
+
 		pair = o->hooks->list;
 		while (pair) {
 			next = pair->next;
 			g_free(pair);
 			pair = next;
 		}
+		e_mutex_destroy(o->hooks->lock);
 		g_free(o->hooks);
 		o->hooks = NULL;
 	}
@@ -739,8 +750,11 @@ static CamelHookList *camel_object_get_hooks(CamelObject *o)
 		if (o->hooks == NULL) {
 			hooks = g_malloc(sizeof(*o->hooks));
 #ifdef ENABLE_THREADS
-			pthread_mutex_init(&hooks->lock, NULL);
+			hooks->lock = e_mutex_new(E_MUTEX_REC);
 #endif
+			hooks->flags = 0;
+			hooks->depth = 0;
+			hooks->list_length = 0;
 			hooks->list = NULL;
 			o->hooks = hooks;
 		}
@@ -750,14 +764,14 @@ static CamelHookList *camel_object_get_hooks(CamelObject *o)
 #endif
 
 #ifdef ENABLE_THREADS
-	pthread_mutex_lock(&o->hooks->lock);
+	e_mutex_lock(o->hooks->lock);
 #endif
 	return o->hooks;	
 }
 
 /* unlock object hooks' list */
 #ifdef ENABLE_THREADS
-#define camel_object_unget_hooks(o) (pthread_mutex_unlock(&(CAMEL_OBJECT(o)->hooks->lock)))
+#define camel_object_unget_hooks(o) (e_mutex_unlock((CAMEL_OBJECT(o)->hooks->lock)))
 #else
 #define camel_object_unget_hooks(o)
 #endif
@@ -789,43 +803,20 @@ camel_object_hook_event (CamelObject * obj, const char * name,
 	pair->name = prepname;	/* effectively static! */
 	pair->func = func;
 	pair->data = data;
+	pair->flags = 0;
 
 	/* get the hook list object, locked, link in new event hook, unlock */
 	hooks = camel_object_get_hooks(obj);
 	pair->next = hooks->list;
 	hooks->list = pair;
+	hooks->list_length++;
 	camel_object_unget_hooks(obj);
-
-#if 0
-	if (obj->event_to_hooklist == NULL)
-		obj->event_to_hooklist =
-			g_hash_table_new (g_str_hash, g_str_equal);
-
-	pair = g_new (CamelHookPair, 1);
-	pair->func = hook;
-	pair->user_data = user_data;
-
-	if (g_hash_table_lookup_extended (obj->event_to_hooklist, name,
-					  &old_name, &old_hooklist)) {
-		hooklist = g_slist_prepend (old_hooklist, pair);
-		g_hash_table_insert (obj->event_to_hooklist, old_name,
-				     hooklist);
-	} else {
-		hooklist = g_slist_prepend (NULL, pair);
-		g_hash_table_insert (obj->event_to_hooklist, g_strdup (name),
-				     hooklist);
-	}
-#endif
 }
 
 void
 camel_object_unhook_event (CamelObject * obj, const char * name,
 			   CamelObjectEventHookFunc func, void *data)
 {
-#if 0
-	GSList *hooklist;
-	GSList *head;
-#endif
 	char *prepname;
 	CamelObjectEventPrepFunc prep;
 	CamelHookList *hooks;
@@ -850,14 +841,23 @@ camel_object_unhook_event (CamelObject * obj, const char * name,
 		return;
 	}
 
-	/* scan hooks for this event, remove it */
+	/* scan hooks for this event, remove it, or flag it if we're busy */
 	hooks = camel_object_get_hooks(obj);
 	parent = (CamelHookPair *)&hooks->list;
 	pair = parent->next;
 	while (pair) {
-		if (pair->name == prepname && pair->func == func && pair->data == data) {
-			parent->next = pair->next;
-			g_free(pair);
+		if (pair->name == prepname
+		    && pair->func == func
+		    && pair->data == data
+		    && (pair->flags & CAMEL_HOOK_PAIR_REMOVED) == 0) {
+			if (hooks->depth > 0) {
+				pair->flags |= CAMEL_HOOK_PAIR_REMOVED;
+				hooks->flags |= CAMEL_HOOK_PAIR_REMOVED;
+			} else {
+				parent->next = pair->next;
+				g_free(pair);
+				hooks->list_length--;
+			}
 			camel_object_unget_hooks(obj);
 			return;
 		}
@@ -868,57 +868,16 @@ camel_object_unhook_event (CamelObject * obj, const char * name,
 
 	g_warning("camel_object_unhook_event: cannot find hook/data pair %p/%p in an instance of `%s' attached to `%s'",
 		  func, data, camel_type_to_name (obj->s.type), name);
-
-#if 0
-	if (obj->event_to_hooklist == NULL) {
-		g_warning
-			("camel_object_unhook_event: trying to unhook `%s' from an instance "
-			 "of `%s' with no hooks attached", name,
-			 camel_type_to_name (obj->s.type));
-		return;
-	}
-
-	hooklist = g_hash_table_lookup (obj->event_to_hooklist, name);
-
-	if (hooklist == NULL) {
-		g_warning
-			("camel_object_unhook_event: trying to unhook `%s' from an instance "
-			 "of `%s' with no hooks attached to that event.",
-			 name, camel_type_to_name (obj->s.type));
-		return;
-	}
-
-	head = hooklist;
-
-	while (hooklist) {
-		CamelHookPair *pair = (CamelHookPair *) hooklist->data;
-
-		if (pair->func == hook && pair->user_data == user_data) {
-			g_free (hooklist->data);
-			head = g_slist_remove_link (head, hooklist);
-			g_slist_free_1 (hooklist);
-			g_hash_table_insert (obj->event_to_hooklist, (char *) name,
-					     head);
-			return;
-		}
-
-		hooklist = hooklist->next;
-	}
-
-	g_warning
-		("camel_object_unhook_event: cannot find hook/data pair %p/%p in an "
-		 "instance of `%s' attached to `%s'", hook, user_data,
-		 camel_type_to_name (obj->s.type), name);
-#endif
 }
 
 void
 camel_object_trigger_event (CamelObject * obj, const char * name, void *event_data)
 {
-	CamelHookPair *pair;
 	CamelObjectEventPrepFunc prep;
 	const char *prepname;
 	CamelHookList *hooks;
+	CamelHookPair *pair, **pairs, *parent;
+	int i, size;
 
 	g_return_if_fail (CAMEL_IS_OBJECT (obj));
 	g_return_if_fail (name);
@@ -940,70 +899,47 @@ camel_object_trigger_event (CamelObject * obj, const char * name, void *event_da
 	camel_object_ref(obj);
 	hooks = camel_object_get_hooks(obj);
 
-	if (prep == NULL_PREP_VALUE || prep(obj, event_data)) {
+	if ((prep == NULL_PREP_VALUE || prep(obj, event_data))
+	    && hooks->list) {
+		/* first, copy the items in the list, and say we're in an event */
+		hooks->depth++;
 		pair = hooks->list;
+		size = 0;
+		pairs = alloca(sizeof(pairs[0]) * hooks->list_length);
 		while (pair) {
 			if (pair->name == prepname)
-				(pair->func) (obj, event_data, pair->data);
-			
+				pairs[size++] = pair;
 			pair = pair->next;
 		}
+
+		/* now execute the events we have, if they haven't been removed during our calls */
+		for (i=0;i<size;i++) {
+			pair = pairs[i];
+			if ((pair->flags & CAMEL_HOOK_PAIR_REMOVED) == 0)
+				(pair->func) (obj, event_data, pair->data);
+		}
+		hooks->depth--;
+
+		/* and if we're out of any events, then clean up any pending removes */
+		if (hooks->depth == 0 && (hooks->flags & CAMEL_HOOK_PAIR_REMOVED)) {
+			parent = (CamelHookPair *)&hooks->list;
+			pair = parent->next;
+			while (pair) {
+				if (pair->flags & CAMEL_HOOK_PAIR_REMOVED) {
+					parent->next = pair->next;
+					g_free(pair);
+					hooks->list_length--;
+				} else {
+					parent = pair;
+				}
+				pair = parent->next;
+			}
+			hooks->flags &= ~CAMEL_HOOK_PAIR_REMOVED;
+		}
 	}
 
 	camel_object_unget_hooks(obj);
 	camel_object_unref(obj);
-
-#if 0
-	if (obj->in_event) {
-		g_warning("camel_object_trigger_event: trying to trigger `%s' in class "
-			  "`%s' while already triggering another event", name,
-			 camel_type_to_name (obj->s.type));
-		return;
-	}
-
-	if (obj->classfuncs->event_to_preplist == NULL) {
-		g_warning
-			("camel_object_trigger_event: trying to trigger `%s' in class "
-			 "`%s' with no defined events.", name,
-			 camel_type_to_name (obj->s.type));
-		return;
-	}
-
-	prep = g_hash_table_lookup (obj->classfuncs->event_to_preplist, name);
-
-	if (prep == NULL) {
-		g_warning
-			("camel_object_trigger_event: trying to trigger undefined "
-			 "event `%s' in class `%s'.", name,
-			 camel_type_to_name (obj->s.type));
-		return;
-	}
-
-	/* Ref so that it can't get destroyed in the event, which would
-	 * be Bad. And it's a valid ref anyway...
-	 */
-
-	camel_object_ref (obj);
-	obj->in_event = 1;
-
-	if ((prep != NULL_PREP_VALUE && !prep (obj, event_data))
-	    || obj->event_to_hooklist == NULL) {
-		obj->in_event = 0;
-		camel_object_unref (obj);
-		return;
-	}
-
-	hooklist = g_hash_table_lookup (obj->event_to_hooklist, name);
-
-	while (hooklist && hooklist->data) {
-		pair = hooklist->data;
-		(pair->func) (obj, event_data, pair->user_data);
-		hooklist = hooklist->next;
-	}
-
-	obj->in_event = 0;
-	camel_object_unref (obj);
-#endif
 }
 
 /* ** Static helpers ****************************************************** */