** Should fix #41629, #41448, et al.

2003-04-22  Not Zed  <NotZed@Ximian.com>

        ** Should fix #41629, #41448, et al.

        * tests/folder/test10.c: a new torture test for object bag
        creation/unreffing.

        * camel-url.c (camel_url_copy): new function to copy a url.

        * camel-object.c (camel_object_bag_new): add arguments for key
        copy and key free functions.  Fixed all callers.
        (camel_object_bag_destroy): fix a memleak, free the bag key.
        (camel_object_bag_get, camel_object_bag_reserve)
        (camel_object_bag_abort, save_bag, save_object): Make the key a
        void type, rather than char *.
        (camel_object_bag_add): As above, and also copy the key.
        (camel_object_bag_remove_unlocked): free the key using
        bag->free_key.

        * camel-session.c (register_provider)
        (camel_session_destroy_provider, get_service): Changed to use an
        object bag instead of a hash table for the service 'cache'.
        (service_cache_remove): Removed, no longer required.

12 files changed, 206 insertions, 50 deletions

diff --git a/camel/ChangeLog b/camel/ChangeLog
index 07da3eecb..4bcdbbec0 100644
--- a/camel/ChangeLog
+++ b/camel/ChangeLog
@@ -1,3 +1,27 @@
+2003-04-22  Not Zed  <NotZed@Ximian.com>
+
+	** Should fix #41629, #41448, et al.
+	
+	* tests/folder/test10.c: a new torture test for object bag
+	creation/unreffing.
+
+	* camel-url.c (camel_url_copy): new function to copy a url.
+
+	* camel-object.c (camel_object_bag_new): add arguments for key
+	copy and key free functions.  Fixed all callers.
+	(camel_object_bag_destroy): fix a memleak, free the bag key.
+	(camel_object_bag_get, camel_object_bag_reserve)
+	(camel_object_bag_abort, save_bag, save_object): Make the key a
+	void type, rather than char *.
+	(camel_object_bag_add): As above, and also copy the key.
+	(camel_object_bag_remove_unlocked): free the key using
+	bag->free_key.
+
+	* camel-session.c (register_provider)
+	(camel_session_destroy_provider, get_service): Changed to use an
+	object bag instead of a hash table for the service 'cache'.
+	(service_cache_remove): Removed, no longer required.
+
 2003-04-21  Jeffrey Stedfast  <fejj@ximian.com>
 
 	* camel-gpg-context.c (gpg_ctx_parse_status): Don't set seen_eof1
diff --git a/camel/camel-data-cache.c b/camel/camel-data-cache.c
index 5d42b9765..db0a0d5e5 100644
--- a/camel/camel-data-cache.c
+++ b/camel/camel-data-cache.c
@@ -79,7 +79,7 @@ static void data_cache_init(CamelDataCache *cdc, CamelDataCacheClass *klass)
 	struct _CamelDataCachePrivate *p;
 
 	p = cdc->priv = g_malloc0(sizeof(*cdc->priv));
-	p->busy_bag = camel_object_bag_new(g_str_hash, g_str_equal);
+	p->busy_bag = camel_object_bag_new(g_str_hash, g_str_equal, g_strdup, g_free);
 }
 
 static void data_cache_finalise(CamelDataCache *cdc)
diff --git a/camel/camel-object.c b/camel/camel-object.c
index 0c08ab7fa..b96e54675 100644
--- a/camel/camel-object.c
+++ b/camel/camel-object.c
@@ -83,6 +83,8 @@ typedef struct _CamelHookPair
 struct _CamelObjectBag {
 	GHashTable *object_table; /* object by key */
 	GHashTable *key_table;	/* key by object */
+	CamelCopyFunc copy_key;
+	GFreeFunc free_key;
 #ifdef ENABLE_THREADS
 	pthread_t owner;	/* the thread that has reserved the bag for a new entry */
 	sem_t reserve_sem;	/* used to track ownership */
@@ -1082,12 +1084,14 @@ camel_object_class_dump_tree(CamelType root)
 	object_class_dump_tree_rec(root, 0);
 }
 
-CamelObjectBag *camel_object_bag_new(GHashFunc hash, GEqualFunc equal)
+CamelObjectBag *camel_object_bag_new(GHashFunc hash, GEqualFunc equal, CamelCopyFunc keycopy, GFreeFunc keyfree)
 {
 	CamelObjectBag *bag;
 
 	bag = g_malloc(sizeof(*bag));
 	bag->object_table = g_hash_table_new(hash, equal);
+	bag->copy_key = keycopy;
+	bag->free_key = keyfree;
 	bag->key_table = g_hash_table_new(NULL, NULL);
 	bag->owner = 0;
 #ifdef ENABLE_THREADS
@@ -1098,7 +1102,7 @@ CamelObjectBag *camel_object_bag_new(GHashFunc hash, GEqualFunc equal)
 }
 
 static void
-save_object(char *key, CamelObject *o, GPtrArray *objects)
+save_object(void *key, CamelObject *o, GPtrArray *objects)
 {
 	g_ptr_array_add(objects, o);
 }
@@ -1112,8 +1116,10 @@ void camel_object_bag_destroy(CamelObjectBag *bag)
 	g_assert(i == 1);
 
 	g_hash_table_foreach(bag->object_table, (GHFunc)save_object, objects);
-	for (i=0;i<objects->len;i++)
+	for (i=0;i<objects->len;i++) {
 		camel_object_bag_remove(bag, objects->pdata[i]);
+		bag->free_key(objects->pdata[i]);
+	}
 	g_ptr_array_free(objects, TRUE);
 	g_hash_table_destroy(bag->object_table);
 	g_hash_table_destroy(bag->key_table);
@@ -1123,12 +1129,12 @@ void camel_object_bag_destroy(CamelObjectBag *bag)
 	g_free(bag);
 }
 
-void camel_object_bag_add(CamelObjectBag *bag, const char *key, void *vo)
+void camel_object_bag_add(CamelObjectBag *bag, const void *key, void *vo)
 {
 	CamelObject *o = vo;
 	CamelHookList *hooks;
 	CamelHookPair *pair;
-	char *k;
+	void *k;
 
 	hooks = camel_object_get_hooks(o);
 	E_LOCK(type_lock);
@@ -1152,7 +1158,7 @@ void camel_object_bag_add(CamelObjectBag *bag, const char *key, void *vo)
 	hooks->list = pair;
 	hooks->list_length++;
 
-	k = g_strdup(key);
+	k = bag->copy_key(key);
 	g_hash_table_insert(bag->object_table, k, vo);
 	g_hash_table_insert(bag->key_table, vo, k);
 
@@ -1167,7 +1173,7 @@ void camel_object_bag_add(CamelObjectBag *bag, const char *key, void *vo)
 	camel_object_unget_hooks(o);
 }
 
-void *camel_object_bag_get(CamelObjectBag *bag, const char *key)
+void *camel_object_bag_get(CamelObjectBag *bag, const void *key)
 {
 	CamelObject *o;
 
@@ -1201,7 +1207,7 @@ void *camel_object_bag_get(CamelObjectBag *bag, const char *key)
 /* After calling reserve, you MUST call bag_abort or bag_add */
 /* Also note that currently you can only reserve a single key
    at any one time in a given thread */
-void *camel_object_bag_reserve(CamelObjectBag *bag, const char *key)
+void *camel_object_bag_reserve(CamelObjectBag *bag, const void *key)
 {
 	CamelObject *o;
 
@@ -1235,7 +1241,7 @@ void *camel_object_bag_reserve(CamelObjectBag *bag, const char *key)
 }
 
 /* abort a reserved key */
-void camel_object_bag_abort(CamelObjectBag *bag, const char *key)
+void camel_object_bag_abort(CamelObjectBag *bag, const void *key)
 {
 #ifdef ENABLE_THREADS
 	g_assert(bag->owner == pthread_self());
@@ -1246,7 +1252,7 @@ void camel_object_bag_abort(CamelObjectBag *bag, const char *key)
 }
 
 static void
-save_bag(char *key, CamelObject *o, GPtrArray *list)
+save_bag(void *key, CamelObject *o, GPtrArray *list)
 {
 	/* we have the refcount lock already */
 	o->ref_count++;
@@ -1272,7 +1278,7 @@ GPtrArray *camel_object_bag_list(CamelObjectBag *bag)
 static void camel_object_bag_remove_unlocked(CamelObjectBag *inbag, CamelObject *o, CamelHookList *hooks)
 {
 	CamelHookPair *pair, *parent;
-	char *oldkey;
+	void *oldkey;
 	CamelObjectBag *bag;
 
 	parent = (CamelHookPair *)&hooks->list;
@@ -1286,7 +1292,7 @@ static void camel_object_bag_remove_unlocked(CamelObjectBag *inbag, CamelObject
 			if (oldkey) {
 				g_hash_table_remove(bag->key_table, o);
 				g_hash_table_remove(bag->object_table, oldkey);
-				g_free(oldkey);
+				bag->free_key(oldkey);
 			}
 			parent->next = pair->next;
 			pair_free(pair);
diff --git a/camel/camel-object.h b/camel/camel-object.h
index 5c5b4ca25..5aa9d3157 100644
--- a/camel/camel-object.h
+++ b/camel/camel-object.h
@@ -216,12 +216,13 @@ void camel_object_free(void *vo, guint32 tag, void *value);
 
 /* for managing bags of weakly-ref'd 'child' objects */
 typedef struct _CamelObjectBag CamelObjectBag;
+typedef void *(*CamelCopyFunc)(const void *vo);
 
-CamelObjectBag *camel_object_bag_new(GHashFunc hash, GEqualFunc equal);
-void *camel_object_bag_get(CamelObjectBag *bag, const char *key);
-void *camel_object_bag_reserve(CamelObjectBag *bag, const char *key);
-void camel_object_bag_add(CamelObjectBag *bag, const char *key, void *o);
-void camel_object_bag_abort(CamelObjectBag *bag, const char *key);
+CamelObjectBag *camel_object_bag_new(GHashFunc hash, GEqualFunc equal, CamelCopyFunc keycopy, GFreeFunc keyfree);
+void *camel_object_bag_get(CamelObjectBag *bag, const void *key);
+void *camel_object_bag_reserve(CamelObjectBag *bag, const void *key);
+void camel_object_bag_add(CamelObjectBag *bag, const void *key, void *o);
+void camel_object_bag_abort(CamelObjectBag *bag, const void *key);
 GPtrArray *camel_object_bag_list(CamelObjectBag *bag);
 void camel_object_bag_remove(CamelObjectBag *bag, void *o);
 void camel_object_bag_destroy(CamelObjectBag *bag);
diff --git a/camel/camel-provider.h b/camel/camel-provider.h
index 5e2bab2ad..edfb42418 100644
--- a/camel/camel-provider.h
+++ b/camel/camel-provider.h
@@ -172,7 +172,7 @@ typedef struct {
 	/* GList of CamelServiceAuthTypes the provider supports */
 	GList *authtypes;
 	
-	GHashTable *service_cache[CAMEL_NUM_PROVIDER_TYPES];
+	CamelObjectBag *service_cache[CAMEL_NUM_PROVIDER_TYPES];
 	
 	GHashFunc url_hash;
 	GCompareFunc url_equal;
diff --git a/camel/camel-session.c b/camel/camel-session.c
index 39ac4167e..520abc26c 100644
--- a/camel/camel-session.c
+++ b/camel/camel-session.c
@@ -109,7 +109,7 @@ camel_session_destroy_provider (gpointer key, gpointer value, gpointer user_data
 
 	for (i = 0; i < CAMEL_NUM_PROVIDER_TYPES; i++) {
 		if (prov->service_cache[i])
-			g_hash_table_destroy (prov->service_cache[i]);
+			camel_object_bag_destroy (prov->service_cache[i]);
 	}
 	return TRUE;
 }
@@ -203,7 +203,8 @@ register_provider (CamelSession *session, CamelProvider *provider)
 
 	for (i = 0; i < CAMEL_NUM_PROVIDER_TYPES; i++) {
 		if (provider->object_types[i])
-			provider->service_cache[i] = g_hash_table_new (provider->url_hash, provider->url_equal);
+			provider->service_cache[i] = camel_object_bag_new (provider->url_hash, provider->url_equal,
+									   (CamelCopyFunc)camel_url_copy, (GFreeFunc)camel_url_free);
 	}
 
 	/* Translate all strings here */
@@ -383,24 +384,6 @@ camel_session_get_provider (CamelSession *session, const char *url_string,
 	return provider;
 }
 
-
-static void
-service_cache_remove (CamelService *service, gpointer event_data, gpointer user_data)
-{
-	CamelSession *session = service->session;
-	CamelProviderType type = GPOINTER_TO_INT (user_data);
-
-	g_return_if_fail (CAMEL_IS_SESSION (session));
-	g_return_if_fail (service != NULL);
-	g_return_if_fail (service->url != NULL);
-
-	CAMEL_SESSION_LOCK(session, lock);
-
-	g_hash_table_remove (service->provider->service_cache[type], service->url);
-
-	CAMEL_SESSION_UNLOCK(session, lock);
-}
-
 static CamelService *
 get_service (CamelSession *session, const char *url_string,
 	     CamelProviderType type, CamelException *ex)
@@ -436,10 +419,9 @@ get_service (CamelSession *session, const char *url_string,
 		camel_url_set_path (url, NULL);
 	
 	/* Now look up the service in the provider's cache */
-	service = g_hash_table_lookup (provider->service_cache[type], url);
+	service = camel_object_bag_reserve(provider->service_cache[type], url);
 	if (service != NULL) {
 		camel_url_free (url);
-		camel_object_ref (CAMEL_OBJECT (service));
 		return service;
 	}
 
@@ -448,15 +430,15 @@ get_service (CamelSession *session, const char *url_string,
 	camel_service_construct (service, session, provider, url, &internal_ex);
 	if (camel_exception_is_set (&internal_ex)) {
 		camel_exception_xfer (ex, &internal_ex);
-		camel_object_unref (CAMEL_OBJECT (service));
+		camel_object_unref (service);
 		service = NULL;
+		camel_object_bag_abort(provider->service_cache[type], url);
 	} else {
-		g_hash_table_insert (provider->service_cache[type], url, service);
-		camel_object_hook_event (CAMEL_OBJECT (service), "finalize",
-					 (CamelObjectEventHookFunc) service_cache_remove,
-					 GINT_TO_POINTER (type));
+		camel_object_bag_add(provider->service_cache[type], url, service);
 	}
-	
+
+	camel_url_free(url);
+
 	return service;
 }
 
diff --git a/camel/camel-store.c b/camel/camel-store.c
index 252456080..2756fbc48 100644
--- a/camel/camel-store.c
+++ b/camel/camel-store.c
@@ -127,7 +127,8 @@ camel_store_init (void *o)
 
 	if (store_class->hash_folder_name) {
 		store->folders = camel_object_bag_new(store_class->hash_folder_name,
-						      store_class->compare_folder_name);
+						      store_class->compare_folder_name,
+						      (CamelCopyFunc)g_strdup, g_free);
 	} else
 		store->folders = NULL;
 	
diff --git a/camel/camel-url.c b/camel/camel-url.c
index 7b1c31db5..78a5beec4 100644
--- a/camel/camel-url.c
+++ b/camel/camel-url.c
@@ -560,3 +560,25 @@ camel_url_equal(const void *v, const void *v2)
 		&& check_equal(u1->query, u2->query)
 		&& u1->port == u2->port;
 }
+
+CamelURL *
+camel_url_copy(const CamelURL *in)
+{
+	CamelURL *out;
+
+	out = g_malloc(sizeof(*out));
+	out->protocol = g_strdup(in->protocol);
+	out->user = g_strdup(in->user);
+	out->authmech = g_strdup(in->authmech);
+	out->passwd = g_strdup(in->passwd);
+	out->host = g_strdup(in->host);
+	out->port = in->port;
+	out->path = g_strdup(in->path);
+	out->params = NULL;
+	if (in->params)
+		g_datalist_foreach(&((CamelURL *)in)->params, copy_param, &out->params);
+	out->query = g_strdup(in->query);
+	out->fragment = g_strdup(in->fragment);
+
+	return out;
+}
diff --git a/camel/camel-url.h b/camel/camel-url.h
index 562befa83..4efa315b3 100644
--- a/camel/camel-url.h
+++ b/camel/camel-url.h
@@ -35,6 +35,7 @@ extern "C" {
 #pragma }
 #endif /* __cplusplus */
 
+/* if this changes, remember to change camel_url_copy */
 typedef struct _CamelURL {
 	char  *protocol;
 	char  *user;
@@ -79,6 +80,7 @@ const char *camel_url_get_param (CamelURL *url, const char *name);
 /* for putting url's into hash tables */
 guint camel_url_hash (const void *v);
 int camel_url_equal(const void *v, const void *v2);
+CamelURL *camel_url_copy(const CamelURL *in);
 
 #ifdef __cplusplus
 }
diff --git a/camel/tests/folder/Makefile.am b/camel/tests/folder/Makefile.am
index c178d5fdb..e49fd92a8 100644
--- a/camel/tests/folder/Makefile.am
+++ b/camel/tests/folder/Makefile.am
@@ -19,8 +19,10 @@ LDADD = \
 check_PROGRAMS =  	\
 	test1	test2	test3	\
 	test4	test5	test6	\
-	test7	test8	test9
+	test7	test8	test9	\
+	test10
 
 TESTS = test1 	test2 	test3 	\
 	test4 	test5 	test6 	\
-	test7	test8	test9
+	test7	test8	test9	\
+	test10
diff --git a/camel/tests/folder/README b/camel/tests/folder/README
index 4fed42141..3ab64ecd1 100644
--- a/camel/tests/folder/README
+++ b/camel/tests/folder/README
@@ -9,3 +9,4 @@ test7	basic folder operations, NNTP
 
 test8	multithreaded folder torture test, local
 test9	filtering
+test10  multithreaded folder/store object bag torture test
diff --git a/camel/tests/folder/test10.c b/camel/tests/folder/test10.c
new file mode 100644
index 000000000..6ae9d9856
--- /dev/null
+++ b/camel/tests/folder/test10.c
@@ -0,0 +1,115 @@
+/* threaded folder testing */
+
+#include <string.h>
+
+#include "camel-test.h"
+#include "folders.h"
+#include "messages.h"
+#include "session.h"
+
+#include <camel/camel-exception.h>
+#include <camel/camel-service.h>
+#include <camel/camel-store.h>
+
+#define MAX_LOOP (10000)
+#define MAX_THREADS (5)
+
+#define d(x) 
+
+#ifndef ENABLE_THREADS
+int main(int argc, char **argv)
+{
+	printf("Test %s is only compiled with threads enabled\n", argv[0]);
+	return 77;
+}
+#else
+
+#include <pthread.h>
+
+
+#define ARRAY_LEN(x) (sizeof(x)/sizeof(x[0]))
+
+static char *local_providers[] = {
+	"mbox",
+	"mh",
+	"maildir"
+};
+
+static char *path;
+static CamelSession *session;
+static int testid;
+
+static void *
+worker(void *d)
+{
+	int i;
+	CamelException *ex = camel_exception_new();
+	CamelStore *store;
+	CamelFolder *folder;
+
+	for (i=0;i<MAX_LOOP;i++) {
+		store = camel_session_get_store(session, path, ex);
+		camel_exception_clear(ex);
+		folder = camel_store_get_folder(store, "testbox", CAMEL_STORE_FOLDER_CREATE, ex);
+		camel_exception_clear(ex);
+		if (testid == 0) {
+			camel_object_unref(folder);
+			camel_object_unref(store);
+		} else {
+			camel_object_unref(store);
+			camel_object_unref(folder);
+		}
+	}
+
+	camel_exception_free(ex);
+
+	return NULL;
+}
+
+int main(int argc, char **argv)
+{
+	CamelException *ex;
+	int i, j;
+	pthread_t threads[MAX_THREADS];
+
+	camel_test_init(argc, argv);
+
+	ex = camel_exception_new();
+
+	/* clear out any camel-test data */
+	system("/bin/rm -rf /tmp/camel-test");
+
+	session = camel_test_session_new ("/tmp/camel-test");
+
+	for (testid=0;testid<2;testid++) {
+		if (testid == 0)
+			camel_test_start("store and folder bag torture test, stacked references");
+		else
+			camel_test_start("store and folder bag torture test, unstacked references");
+
+		for (j=0;j<ARRAY_LEN(local_providers);j++) {
+
+			camel_test_push("provider %s", local_providers[j]);
+			path = g_strdup_printf("%s:///tmp/camel-test/%s", local_providers[j], local_providers[j]);
+		
+			for (i=0;i<MAX_THREADS;i++)
+				pthread_create(&threads[i], 0, worker, NULL);
+			
+			for (i=0;i<MAX_THREADS;i++) 
+				pthread_join(threads[i], NULL);
+			
+			test_free(path);
+
+			camel_test_pull();
+		}
+
+		camel_test_end();
+	}
+
+	camel_object_unref((CamelObject *)session);
+	camel_exception_free(ex);
+
+	return 0;
+}
+
+#endif /* ENABLE_THREADS */