On Di, 2008-09-02 at 19:04 +0200, Philipp Riegger wrote: > On Mon, 2008-09-01 at 19:55 +0200, Patrick Ohly wrote: > > * 499932: [Bug 499932] Not deleting from e-mail server after > > specified time > > I applied this to the gentoo evolution-data-server ebuild and had some > instability. Evolution died after a timeout in one of my pop3 accounts > (server could not be found/reached). Be sure, to double check that.
Thanks a lot for reporting this! I didn't notice that when trying out trunk. Now I had a look at the actual patch and found that it has a (potentially huge?) memory leak: in pop3_get_message_time_from_cache() after camel_data_cache_get() the returned stream already has a refcount of 1, but the code did another camel_object_ref(). Therefore the camel_object_unref() later on only reduced the refcount to 1 and the stream was never freed. The stream was also not freed if reading from it failed or didn't contain the expected content - not sure whether that really happens. Attached is the original and the revised patch. It works for me on 2.22.x. Okay to commit on 2.22.x and to fix the trunk accordingly? -- Bye, Patrick Ohly -- [EMAIL PROTECTED] http://www.estamos.de/
Index: camel/providers/pop3/ChangeLog =================================================================== --- camel/providers/pop3/ChangeLog (revision 8571) +++ camel/providers/pop3/ChangeLog (working copy) @@ -1,3 +1,11 @@ +2008-03-12 Milan Crha <[EMAIL PROTECTED]> + + ** Fix for bug #514827 + + * camel-pop3-folder.c: (pop3_get_message_time_from_cache), + (camel_pop3_delete_old): POP3 folder doesn't have a summary, + it has its own cache, so read message time from it. + 2008-02-18 Chenthill Palanisamy <[EMAIL PROTECTED]> * camel-pop3-store.c (connect_to_server): Fix for some Index: camel/providers/pop3/camel-pop3-folder.c =================================================================== --- camel/providers/pop3/camel-pop3-folder.c (revision 8571) +++ camel/providers/pop3/camel-pop3-folder.c (working copy) @@ -356,6 +356,49 @@ pop3_sync (CamelFolder *folder, gboolean camel_pop3_store_expunge (pop3_store, ex); } +static gboolean +pop3_get_message_time_from_cache (CamelFolder *folder, const char *uid, time_t *message_time) +{ + CamelPOP3Store *pop3_store; + CamelStream *stream = NULL; + char buffer[1]; + gboolean res = FALSE; + + g_return_val_if_fail (folder != NULL, FALSE); + g_return_val_if_fail (uid != NULL, FALSE); + g_return_val_if_fail (message_time != NULL, FALSE); + + pop3_store = CAMEL_POP3_STORE (folder->parent_store); + + g_return_val_if_fail (pop3_store->cache != NULL, FALSE); + + if ((stream = camel_data_cache_get (pop3_store->cache, "cache", uid, NULL)) != NULL + && camel_stream_read (stream, buffer, 1) == 1 + && buffer[0] == '#') { + CamelMimeMessage *message; + + camel_object_ref ((CamelObject *)stream); + + message = camel_mime_message_new (); + if (camel_data_wrapper_construct_from_stream ((CamelDataWrapper *)message, stream) == -1) { + g_warning (_("Cannot get message %s: %s"), uid, g_strerror (errno)); + camel_object_unref ((CamelObject *)message); + message = NULL; + } + + if (message) { + res = TRUE; + *message_time = message->date + message->date_offset; + + camel_object_unref ((CamelObject *)message); + } + + camel_object_unref ((CamelObject *)stream); + } + + return res; +} + int camel_pop3_delete_old(CamelFolder *folder, int days_to_delete, CamelException *ex) { @@ -363,8 +406,7 @@ camel_pop3_delete_old(CamelFolder *folde CamelPOP3FolderInfo *fi; int i; CamelPOP3Store *pop3_store; - time_t temp; - CamelMessageInfo *minfo; + time_t temp, message_time; pop3_folder = CAMEL_POP3_FOLDER (folder); pop3_store = CAMEL_POP3_STORE (CAMEL_FOLDER(pop3_folder)->parent_store); @@ -374,10 +416,8 @@ camel_pop3_delete_old(CamelFolder *folde for (i = 0; i < pop3_folder->uids->len; i++) { fi = pop3_folder->uids->pdata[i]; - minfo = camel_folder_get_message_info (folder, fi->uid); d(printf("%s(%d): fi->uid=[%s]\n", __FILE__, __LINE__, fi->uid)); - if(minfo) { - time_t message_time = ((CamelMessageInfoBase *)minfo)->date_received; + if (pop3_get_message_time_from_cache (folder, fi->uid, &message_time)) { double time_diff = difftime(temp,message_time); int day_lag = time_diff/(60*60*24); @@ -407,8 +447,6 @@ camel_pop3_delete_old(CamelFolder *folde camel_data_cache_remove(pop3_store->cache, "cache", fi->uid, NULL); } } - /* free message - not used anymore */ - camel_folder_free_message_info (folder, minfo); } }
Index: camel-pop3-folder.c =================================================================== --- camel-pop3-folder.c (revision 9476) +++ camel-pop3-folder.c (working copy) @@ -356,6 +356,48 @@ camel_pop3_store_expunge (pop3_store, ex); } +static gboolean +pop3_get_message_time_from_cache (CamelFolder *folder, const char *uid, time_t *message_time) +{ + CamelPOP3Store *pop3_store; + CamelStream *stream = NULL; + char buffer[1]; + gboolean res = FALSE; + + g_return_val_if_fail (folder != NULL, FALSE); + g_return_val_if_fail (uid != NULL, FALSE); + g_return_val_if_fail (message_time != NULL, FALSE); + + pop3_store = CAMEL_POP3_STORE (folder->parent_store); + + g_return_val_if_fail (pop3_store->cache != NULL, FALSE); + + if ((stream = camel_data_cache_get (pop3_store->cache, "cache", uid, NULL)) != NULL + && camel_stream_read (stream, buffer, 1) == 1 + && buffer[0] == '#') { + CamelMimeMessage *message; + + message = camel_mime_message_new (); + if (camel_data_wrapper_construct_from_stream ((CamelDataWrapper *)message, stream) == -1) { + g_warning (_("Cannot get message %s: %s"), uid, g_strerror (errno)); + camel_object_unref ((CamelObject *)message); + message = NULL; + } + + if (message) { + res = TRUE; + *message_time = message->date + message->date_offset; + + camel_object_unref ((CamelObject *)message); + } + } + + if (stream) { + camel_object_unref (stream); + } + return res; +} + int camel_pop3_delete_old(CamelFolder *folder, int days_to_delete, CamelException *ex) { @@ -363,8 +405,7 @@ CamelPOP3FolderInfo *fi; int i; CamelPOP3Store *pop3_store; - time_t temp; - CamelMessageInfo *minfo; + time_t temp, message_time; pop3_folder = CAMEL_POP3_FOLDER (folder); pop3_store = CAMEL_POP3_STORE (CAMEL_FOLDER(pop3_folder)->parent_store); @@ -374,10 +415,8 @@ for (i = 0; i < pop3_folder->uids->len; i++) { fi = pop3_folder->uids->pdata[i]; - minfo = camel_folder_get_message_info (folder, fi->uid); d(printf("%s(%d): fi->uid=[%s]\n", __FILE__, __LINE__, fi->uid)); - if(minfo) { - time_t message_time = ((CamelMessageInfoBase *)minfo)->date_received; + if (pop3_get_message_time_from_cache (folder, fi->uid, &message_time)) { double time_diff = difftime(temp,message_time); int day_lag = time_diff/(60*60*24); @@ -407,8 +446,6 @@ camel_data_cache_remove(pop3_store->cache, "cache", fi->uid, NULL); } } - /* free message - not used anymore */ - camel_folder_free_message_info (folder, minfo); } }
_______________________________________________ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers