[Evolution] Re: [Evolution-hackers] SIGSEGV on setting label - gdb output and analysis

[Jeffrey Stedfast <fejj ximian com>]

On Sun, 2003-01-05 at 08:04, Mika Liljeberg wrote:
> Hi,
>
> I have further information on the following bug:
>
> http://bugzilla.ximian.com/show_bug.cgi?id=34164
>
> Observed on Evo 1.2.0. I got this by selecting all messages in a vfolder
> and trying to set a label on them.
>
> I managed to trap this in gdb. If I read the code right, it looks like
> there is a thread locking problem in e-util/e-msgport.c:e_thread_put():
>
>         switch(e->type) {
>         case E_THREAD_QUEUE:
>                 /* if the queue is full, lose this new addition */
> ==>             if (e_dlist_length(&e->server_port->queue) < e->queue_limit) {
>                         e_msgport_put(e->server_port, msg);
>                 } else {
>                         printf("queue limit reached, dropping new message\n");
>                         dmsg = msg;
>                 }
>                 break;
>         case E_THREAD_DROP:
>                 /* if the queue is full, lose the oldest (unprocessed) message */
> ==>             if (e_dlist_length(&e->server_port->queue) < e->queue_limit) {
>                         e_msgport_put(e->server_port, msg);
>                 } else {
>                         printf("queue limit reached, dropping old message\n");
>                         e_msgport_put(e->server_port, msg);
>                         dmsg = e_msgport_get(e->server_port);
>                 }
>                 break;
>
> As you can see, there are two calls to e_dlist_length() to find out the
> message queue length without locking it first. e_dlist_length() iterates
> the whole message queue, which might be modified simultaneously by
> another thread.

except that it can't be modified by another thread because a few lines
above the code you pasted, there is a:

pthread_mutex_lock (&e->mutex);

Note that (assuming I am reading the code correctly, anyway) the EThread
(e) *owns* the EMsgPort (e->server_port). This means that so long as we
don't access the EThread's EMsgPort directly (which we can't, note that
EThread is a private structure) or without first locking e->mutex, there
can be no "thread locking problems".

Simply put: so long as e->mutex is locked, accessing
e->server_port->queue is safe. There is no need to lock
e->server_port->lock.

So, this means that the only way problems could arise is if we modify
e->server_port->queue outside of a e->mutex lock. I scanned the code and
could not find evidence that this could occur.

I think that the corruption to this list may be more likely to just be
random memory corruption than a bug in the e-msgport.c code, but I'll
wait to see what Michael Zucchi says (he's the one that wrote this
code).

Jeff

-- 
Jeffrey Stedfast
Evolution Hacker - Ximian, Inc.
fejj ximian com  - www.ximian.com