[Evolution-hackers] Two multithread problems: memorychunk and message-list

[jeff <Jeff Cai Sun COM>]

On Mon, 2006-01-23 at 18:21, Parthasarathi Susarla wrote:
> hey jeff,
> could you send the details regarding the issue you were pointing out on
> the IRC, i was kinda away then - so could not actually catch up.
> 
> 
> Thanks and CHeers,
> partha
> 

Hi, partha

This is a problem in evolution 1.4, a bit older version. :(
But I doubt it may be an implict problem for version 2.x.
I meet a crash while deleting a large amount of mails in an IMAP folder.
I can reproduce it by pressing 'Delete' Key constantly to delete all
mails
in an folder. 
Following is the trace

[1] e_memchunk_alloc(m = 0x2da0d0), line 156 in "e-memory.c"
[2] e_mempool_new(blocksize = 512, threshold = 256, flags =
E_MEMPOOL_ALIGN_BYTE), line 414 in "e-memory.c"
[3] camel_folder_search_execute_expression(search = 0x3232b0, expr =
0x2e20a8 " (and\n  \n       (match-all (< (get-received-date) (-
(get-current-date) 86400)))\n    \n  \n       (match-all (>
(get-received-date) (- (get-current-date) 172800)))\n    \n  )\n", ex =
(nil)), line 371 in "camel-folder-search.c"
[4] imap_search_by_uids(folder = 0x3178a0, expression = 0x2e20a8 "
(and\n  \n       (match-all (< (get-received-date) (- (get-current-date)
86400)))\n    \n  \n       (match-all (> (get-received-date) (-
(get-current-date) 172800)))\n    \n  )\n", uids = 0xb4d094, ex =
(nil)), line 1596 in "camel-imap-folder.c".

then some codes of e_memchunk_alloc
       f = m->free;
        if (f) {
                f->atoms--;
                if (f->atoms > 0) {
                        mem = ((char *)f) + (f->atoms*m->atomsize);
                } else {
                        mem = f;
                        m->free = m->free->next;
                }
                pthread_mutex_unlock( &g_mutex );
                return mem;
        } else {
                b = g_malloc(m->blocksize * m->atomsize);
                g_ptr_array_add(m->blocks, b);


I check the m, it is as this
*m = {
    blocksize = 8U
    atomsize  = 20U
    blocks    = 0xeaaec
    free      = 0x814170
} 

but, i find that 'f' has changed to 0x300.

This means other threads have changed it.
Then I do a test:
i lock all operations to memchunk, such as e_memchunk_alloc,
e_memchunk_free, e
_memchunk_empty etc. This bug can't be reproduced.

I looked it up in e-util/ChangeLog in the trunk
I got the following infomation:

2004-01-05  Not Zed  <NotZed Ximian com>

        * e-memory.c (e_mempool_destroy): Fix from Zan Lynx
        <zlynx acm org> to lock the memchunk before freeing the pool
        header.
Then i tried as the change, but evolution can still crash.

Note, the bug can be reproduced in a multiprocessor machine.

I've tested it on evolution 2.4, the bug can't be reproduced. I think
that is because
the message is put on the queue in an interval of 500ms which reduces
the risking resource among threads. But sometimes, evolution will
crash and give a strange trace, someone has filed two bugs concerning
with memory management,
i don't know whether this will relate with those crashes. Could you
review the codes about the memory chunk and
give me an answer?

Additionally, I ever send a mail to evolution-hackers about message-list
lock but 
don't get any response yet. I also attach that mail at following, please
review it.

************************************************************************************


     From: 
jeff cai
<Jeff Cai Sun COM>
       To: 
evolution-hackers gnome org
  Subject: 
[Evolution-hackers] A multithread problem
     Date: 
Thu, 12 Jan
2006
21:52:03
+0800

Hi,all
I found an implict multithread problem. I want to see
whether anybody could give me more information about it.

In function message-list.c:mail_regen_list,
a new message is created and put into a thread queue,
meanwhile, the message is also prepended to a regen list of the message
list.
But, the regen list is not protected by a mutex which maybe lead to a
severe problem because
it will be changed in thread function "regen_list_free" and
"regen_list_regened",
is this a bug of evolution?



/* if we're busy already kick off timeout processing, so normal updates
are immediate */
        if (ml->regen == NULL)
*****************************************************************
the new message is put into a thread queue
****************************************************************
                ml_regen_timeout(m); 
        else {
                ml->regen_timeout_msg = m;
                ml->regen_timeout_id = g_timeout_add(500,
(GSourceFunc)ml_regen_timeout, m);
        }



static gboolean
ml_regen_timeout(struct _regen_list_msg *m)
{
        e_profile_event_emit("list.regenerate", m->folder->full_name,
0);

        ***************************************************
        This code is not protected
        ***************************************************
        m->ml->regen = g_list_prepend(m->ml->regen, m);                 
        /* TODO: we should manage our own thread stuff, would make
cancelling outstanding stuff easier */
        e_thread_put (mail_thread_queued, (EMsg *)m);

        m->ml->regen_timeout_msg = NULL;
        m->ml->regen_timeout_id = 0;

        return FALSE;
}

regen_list_free (struct _mail_msg *mm)
{
        struct _regen_list_msg *m = (struct _regen_list_msg *)mm;
        int i;

        e_profile_event_emit("list.regenerated", m->folder->full_name,
0);

        if (m->summary) {
                for (i = 0; i < m->summary->len; i++)
                        camel_folder_free_message_info (m->folder,
m->summary->pdata[i]);
                g_ptr_array_free (m->summary, TRUE);
        }

        if (m->tree)
                camel_folder_thread_messages_unref (m->tree);

        g_free (m->search);
        g_free (m->hideexpr);

        camel_object_unref (m->folder);

        if (m->changes)
                camel_folder_change_info_free (m->changes);

        /* we have to poke this here as well since we might've been
cancelled and regened wont get called */
        *****************************************************
        This code is not protected
        *****************************************************
        m->ml->regen = g_list_remove(m->ml->regen, m);

        g_object_unref(m->ml);
}


Jeff Cai