Reusing message info instances after remapping

[Philip Van Hoof <spam pvanhoof be>]

This is a little bit a difficult to explain subject. Rather technical.

What will happen on camel_folder_summary_save is that the mmap will
reload itself.

This reloading currently effectively makes all the message-info
instances being used by the TnyCamelHeader instances incorrect and
hazardous.

That's why it's not possible to expunge a folder while still having
headers being managed. I added an assertion check for that. It will
simply deny the expunge request.

That's because expunging means saving the summary. And saving the
summary means reloading the mmap.

This is a rather icky limitation for the developer's point of view: he
has to close the folder completely (destroy all header instances) before
he can expunge it.


This patch is a first attempt into allowing expunges to happen on open
folders. Headers of removed messages will simply end up pointing to a
NULL pointer for their message-info instance. And existing message-info
instances will be reused by the code that parses trough the mmapped data
and assigns the pointers which are defined in the structure.

It currently does this by searching for an original one that had the
same uid, and then in stead of destroying the instance, reusing it
(correcting it with the new mmap offsets after the file is mmapped).

There are a few glitches in it (it does work, but some headers aren't
found and things like that).

I would love others to take a look at this. It's very tricky things that
I have to do. I will probably get it 100% perfect during the weekend or
something. But feel free to send me improved versions of this patch.

I do warn that it's icky tricky code. A bit hard to get it right. You
need to fully understand how I did the mmap trick. Stuff like that.

Nevertheless I'm very interested in the opinions of you guys.


ps. YES, I know I should use a factory. And feel free to put that in the
patch if you believe that will give you more control over the decision
of recreation or reuse of an instance. Do note that each allocation here
is extremely expensive: you need to repeat that allocation FOR EVERY
header (so 50,000 headers means 50,000 such allocations. Which becomes
quickly extremely expensive). (and I know that a factory doesn't imply
these extra allocations, just keep it in the back of your head).

ps. There's a hashtable called messages_uid which might be useful for
replacing the search code. On the other hand I'm trying to remove that
hashtable too (it's needlessly consuming memory for a feature that
tinymail does not really use).


Go for it :). Don't forget to have fun


-- 
Philip Van Hoof, software developer
home: me at pvanhoof dot be
gnome: pvanhoof at gnome dot org
work: vanhoof at x-tend dot be
blog: http://pvanhoof.be/blog

Index: tny-camel-folder.c
===================================================================
--- tny-camel-folder.c	(revision 1112)
+++ tny-camel-folder.c	(working copy)
@@ -273,7 +273,7 @@
 	TnyCamelFolderPriv *priv = TNY_CAMEL_FOLDER_GET_PRIVATE (self);
 	CamelException ex = CAMEL_EXCEPTION_INITIALISER;
 
-	if (priv->headers_managed > 0)
+	if (FALSE && priv->headers_managed > 0)
 	{
 		g_critical ("Request to expunge a folder denied: there are still "
 			"header instances of this folder active. Destroy them "
Index: camel-lite/camel/providers/imap/camel-imap-summary.c
===================================================================
--- camel-lite/camel/providers/imap/camel-imap-summary.c	(revision 1112)
+++ camel-lite/camel/providers/imap/camel-imap-summary.c	(working copy)
@@ -40,7 +40,7 @@
 static int summary_header_load (CamelFolderSummary *);
 static int summary_header_save (CamelFolderSummary *, FILE *);
 
-static CamelMessageInfo *message_info_load (CamelFolderSummary *s);
+static CamelMessageInfo *message_info_load (CamelFolderSummary *s, gboolean *must_add);
 static int message_info_save (CamelFolderSummary *s, FILE *out,
 			      CamelMessageInfo *info);
 static gboolean info_set_user_tag(CamelMessageInfo *info, const char *name, const char  *value);
@@ -206,12 +206,12 @@
 }
 
 static CamelMessageInfo *
-message_info_load (CamelFolderSummary *s)
+message_info_load (CamelFolderSummary *s, gboolean *must_add)
 {
 	CamelMessageInfo *info;
 	CamelImapMessageInfo *iinfo;
 
-	info = camel_imap_summary_parent->message_info_load (s);
+	info = camel_imap_summary_parent->message_info_load (s, must_add);
 	iinfo = (CamelImapMessageInfo*)info;
 
 	if (info) {
Index: camel-lite/camel/providers/imapp/camel-imapp-summary.c
===================================================================
--- camel-lite/camel/providers/imapp/camel-imapp-summary.c	(revision 1112)
+++ camel-lite/camel/providers/imapp/camel-imapp-summary.c	(working copy)
@@ -40,7 +40,7 @@
 static int summary_header_load(CamelFolderSummary *);
 static int summary_header_save(CamelFolderSummary *, FILE *);
 
-static CamelMessageInfo *message_info_load(CamelFolderSummary *s);
+static CamelMessageInfo *message_info_load(CamelFolderSummary *s, gboolean *must_add);
 static int message_info_save(CamelFolderSummary *s, FILE *out, CamelMessageInfo *info);
 
 static void camel_imapp_summary_class_init(CamelIMAPPSummaryClass *klass);
@@ -159,12 +159,12 @@
 
 
 static CamelMessageInfo *
-message_info_load(CamelFolderSummary *s)
+message_info_load(CamelFolderSummary *s, gboolean *must_add)
 {
 	CamelMessageInfo *info;
 	CamelIMAPPMessageInfo *iinfo;
 
-	info = camel_imapp_summary_parent->message_info_load(s);
+	info = camel_imapp_summary_parent->message_info_load(s, must_add);
 	if (info) {
 		unsigned char *ptrchr = s->filepos;
 		iinfo =(CamelIMAPPMessageInfo *)info;
Index: camel-lite/camel/providers/local/camel-mbox-summary.c
===================================================================
--- camel-lite/camel/providers/local/camel-mbox-summary.c	(revision 1112)
+++ camel-lite/camel/providers/local/camel-mbox-summary.c	(working copy)
@@ -57,7 +57,7 @@
 
 static CamelMessageInfo * message_info_new_from_header(CamelFolderSummary *, struct _camel_header_raw *);
 static CamelMessageInfo * message_info_new_from_parser(CamelFolderSummary *, CamelMimeParser *);
-static CamelMessageInfo * message_info_load (CamelFolderSummary *);
+static CamelMessageInfo * message_info_load (CamelFolderSummary *, gboolean *);
 static int		  message_info_save (CamelFolderSummary *, FILE *, CamelMessageInfo *);
 static int 		  meta_message_info_save(CamelFolderSummary *s, FILE *out_meta, FILE *out, CamelMessageInfo *mi);
 /*static void		  message_info_free (CamelFolderSummary *, CamelMessageInfo *);*/
@@ -378,13 +378,13 @@
 
 
 static CamelMessageInfo *
-message_info_load(CamelFolderSummary *s)
+message_info_load(CamelFolderSummary *s, gboolean *must_add)
 {
 	CamelMessageInfo *mi;
 
 	io(printf("loading mbox message info\n"));
 
-	mi = ((CamelFolderSummaryClass *)camel_mbox_summary_parent)->message_info_load(s);
+	mi = ((CamelFolderSummaryClass *)camel_mbox_summary_parent)->message_info_load(s, must_add);
 	if (mi) {
 		CamelMboxMessageInfo *mbi = (CamelMboxMessageInfo *)mi;
 
Index: camel-lite/camel/providers/local/camel-maildir-summary.c
===================================================================
--- camel-lite/camel/providers/local/camel-maildir-summary.c	(revision 1112)
+++ camel-lite/camel/providers/local/camel-maildir-summary.c	(working copy)
@@ -48,7 +48,7 @@
 
 #define CAMEL_MAILDIR_SUMMARY_VERSION (0x2000)
 
-static CamelMessageInfo *message_info_load(CamelFolderSummary *s);
+static CamelMessageInfo *message_info_load(CamelFolderSummary *s, gboolean *must_add);
 static CamelMessageInfo *message_info_new_from_header(CamelFolderSummary *, struct _camel_header_raw *);
 static void message_info_free(CamelFolderSummary *, CamelMessageInfo *mi);
 
@@ -385,12 +385,12 @@
 }
 
 static CamelMessageInfo *
-message_info_load(CamelFolderSummary *s)
+message_info_load(CamelFolderSummary *s, gboolean *must_add)
 {
 	CamelMessageInfo *mi;
 	CamelMaildirSummary *mds = (CamelMaildirSummary *)s;
 
-	mi = ((CamelFolderSummaryClass *) parent_class)->message_info_load(s);
+	mi = ((CamelFolderSummaryClass *) parent_class)->message_info_load(s, must_add);
 	if (mi) {
 		char *name;
 
Index: camel-lite/camel/camel-folder-summary.c
===================================================================
--- camel-lite/camel/camel-folder-summary.c	(revision 1112)
+++ camel-lite/camel/camel-folder-summary.c	(working copy)
@@ -99,7 +99,7 @@
 static CamelMessageInfo * message_info_new_from_header(CamelFolderSummary *, struct _camel_header_raw *);
 static CamelMessageInfo * message_info_new_from_parser(CamelFolderSummary *, CamelMimeParser *);
 static CamelMessageInfo * message_info_new_from_message(CamelFolderSummary *s, CamelMimeMessage *msg);
-static CamelMessageInfo * message_info_load(CamelFolderSummary *);
+static CamelMessageInfo * message_info_load(CamelFolderSummary *, gboolean *must_add);
 static int		  message_info_save(CamelFolderSummary *, FILE *, CamelMessageInfo *);
 static void		  message_info_free(CamelFolderSummary *, CamelMessageInfo *);
 
@@ -145,8 +145,8 @@
 	s->flags = 0;
 	s->time = 0;
 	s->nextuid = 1;
+	s->in_reload = FALSE;
 
-
 	s->messages = g_ptr_array_new();
 	s->messages_uid = g_hash_table_new(g_str_hash, g_str_equal);
 	
@@ -181,17 +181,18 @@
 
 	p = _PRIVATE(s);
 
-	camel_folder_summary_clear(s);
+	/* camel_folder_summary_clear(s); */
+
 	if (s->file)
 		g_mapped_file_free (s->file);
 	s->file = NULL;
-
+/*
 	g_ptr_array_foreach (s->messages, foreach_msginfo, (gpointer)s->message_info_size);
 	if (s->messages->len > 0)
 		g_ptr_array_remove_range (s->messages, 0, s->messages->len-1);
 	g_hash_table_foreach_remove (s->messages_uid, always_true, NULL);
 	g_hash_table_foreach(p->filter_charset, free_o_name, 0);
-	
+
 	if (p->filter_index)
 		camel_object_unref((CamelObject *)p->filter_index);
 	if (p->filter_64)
@@ -209,6 +210,7 @@
 		camel_object_unref((CamelObject *)p->filter_stream);
 	if (p->index)
 		camel_object_unref((CamelObject *)p->index);
+*/
 }
 
 static void
@@ -610,7 +612,6 @@
 	int i;
 	CamelMessageInfo *mi;
 
-
 	if (s->summary_path == NULL || !g_file_test (s->summary_path, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR))
 		return 0;
 
@@ -626,7 +627,8 @@
 
 	/* now read in each message ... */
 	for (i=0;i<s->saved_count;i++) {
-		mi = ((CamelFolderSummaryClass *)(CAMEL_OBJECT_GET_CLASS(s)))->message_info_load(s);
+		gboolean must_add = FALSE;
+		mi = ((CamelFolderSummaryClass *)(CAMEL_OBJECT_GET_CLASS(s)))->message_info_load(s, &must_add);
 
 		if (mi == NULL)
 			goto error;
@@ -641,7 +643,8 @@
 			}
 		}
 
-		camel_folder_summary_mmap_add(s, mi);
+		if (must_add)
+			camel_folder_summary_mmap_add(s, mi);
 	}
 
 
@@ -741,13 +744,20 @@
 
 	for (i = 0; i < count; i++) {
 		mi = s->messages->pdata[i];
+
 		if (((CamelFolderSummaryClass *)(CAMEL_OBJECT_GET_CLASS (s)))->message_info_save (s, out, mi) == -1)
 			goto exception;
-		
+
 		if (s->build_content) {
 			if (perform_content_info_save (s, out, ((CamelMessageInfoBase *)mi)->content) == -1)
 				goto exception;
 		}
+
+		if (! (((CamelMessageInfoBase*)mi)->flags & CAMEL_MESSAGE_INFO_UID_NEEDS_FREE))
+		{
+			mi->uid = g_strdup (mi->uid);
+			((CamelMessageInfoBase*)mi)->flags |= CAMEL_MESSAGE_INFO_UID_NEEDS_FREE;
+		}
 	}
 
 	if (fflush (out) != 0 || fsync (fileno (out)) == -1)
@@ -757,6 +767,8 @@
 	
 	fclose (out);
 
+	s->in_reload = TRUE;
+
 	camel_folder_summary_unload_mmap (s);
 #ifdef G_OS_WIN32
 	g_unlink(s->summary_path);
@@ -769,6 +781,8 @@
 	}
 	camel_folder_summary_load (s);
 
+	s->in_reload = FALSE;
+
 	s->flags &= ~CAMEL_SUMMARY_DIRTY;
 
 	g_mutex_unlock (s->dump_lock);
@@ -1869,26 +1883,65 @@
 
 
 static CamelMessageInfo *
-message_info_load(CamelFolderSummary *s)
+message_info_load(CamelFolderSummary *s, gboolean *must_add)
 {
-	CamelMessageInfoBase *mi;
+	CamelMessageInfoBase *mi = NULL;
 	guint count, len;
 	unsigned char *ptrchr = s->filepos;
 	unsigned int i;
+	gchar *theuid;
 
 #ifndef NON_TINYMAIL_FEATURES
 	unsigned int size;
 #endif
 
-	mi = (CamelMessageInfoBase *)camel_message_info_new(s);
-
 	io(printf("Loading message info\n"));
 
 	ptrchr = camel_file_util_mmap_decode_uint32 (ptrchr, &len, TRUE);
 	if (len) 
-		mi->uid = (char*)ptrchr;
+		theuid = (char*)ptrchr;
 	ptrchr += len;
 
+	if (!s->in_reload)
+	{
+		mi = (CamelMessageInfoBase *)camel_message_info_new(s);
+		*must_add = TRUE;
+	} else
+	{
+		guint count = s->messages->len;
+
+		for (i = 0; i < count; i++) 
+		{
+			CamelMessageInfoBase *ni = s->messages->pdata[i];
+
+			printf ("Looking for (%s) - (%s) - %d of %d\n",
+				theuid, ni->uid, i, count);
+
+			if ((ni->flags & CAMEL_MESSAGE_INFO_UID_NEEDS_FREE) && ni->uid)
+			{
+				if (strcmp (ni->uid, theuid) == 0)
+				{
+					printf ("Found %s\n", theuid);
+					*must_add = FALSE;
+					mi = ni;
+					g_free (ni->uid);
+					ni->flags &= ~CAMEL_MESSAGE_INFO_UID_NEEDS_FREE;
+					break;
+				}
+			}
+		}
+	}
+
+	i = 0;
+
+	if (!mi)
+	{
+		mi = (CamelMessageInfoBase *)camel_message_info_new(s);
+		*must_add = TRUE;
+	}
+
+	mi->uid = theuid;
+
 	ptrchr = camel_file_util_mmap_decode_uint32 (ptrchr, &mi->flags, FALSE);
 
 	mi->flags &= ~CAMEL_MESSAGE_INFO_UID_NEEDS_FREE;
Index: camel-lite/camel/camel-folder-summary.h
===================================================================
--- camel-lite/camel/camel-folder-summary.h	(revision 1112)
+++ camel-lite/camel/camel-folder-summary.h	(working copy)
@@ -235,7 +235,7 @@
 	char *summary_path;
 	gboolean build_content;	/* do we try and parse/index the content, or not? */
 
-	GPtrArray *messages;	/* CamelMessageInfo's */
+	GPtrArray *messages; /* CamelMessageInfo's */
 	GHashTable *messages_uid; /* CamelMessageInfo's by uid */
 
 	struct _CamelFolder *folder; /* parent folder, for events */
@@ -244,6 +244,7 @@
 	GMappedFile *file;
 	unsigned char *filepos;
 	GMutex *dump_lock;
+	gboolean in_reload;
 };
 
 struct _CamelFolderSummaryClass {
@@ -257,7 +258,7 @@
 	CamelMessageInfo * (*message_info_new_from_header)(CamelFolderSummary *, struct _camel_header_raw *);
 	CamelMessageInfo * (*message_info_new_from_parser)(CamelFolderSummary *, CamelMimeParser *);
 	CamelMessageInfo * (*message_info_new_from_message)(CamelFolderSummary *, CamelMimeMessage *);
-	CamelMessageInfo * (*message_info_load)(CamelFolderSummary *);
+	CamelMessageInfo * (*message_info_load)(CamelFolderSummary *, gboolean *must_add);
  	int		   (*message_info_save)(CamelFolderSummary *, FILE *, CamelMessageInfo *);
 	int		   (*meta_message_info_save)(CamelFolderSummary *, FILE *, FILE *, CamelMessageInfo *);