WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH, v2] xenstore: Fix deadlock in xs_read_watch

To: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH, v2] xenstore: Fix deadlock in xs_read_watch
From: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Date: Mon, 30 Aug 2010 11:05:30 -0400
Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 30 Aug 2010 08:06:24 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <alpine.DEB.2.00.1008301443480.2545@kaball-desktop>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: National Security Agency
References: <4C76D9C3.5020600@xxxxxxxxxxxxx> <alpine.DEB.2.00.1008301413080.2545@kaball-desktop> <4C7BB3E6.1080406@xxxxxxxxxxxxx> <alpine.DEB.2.00.1008301443480.2545@kaball-desktop>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.8) Gecko/20100806 Fedora/3.1.2-1.fc13 Thunderbird/3.1.2
On 08/30/2010 09:52 AM, Stefano Stabellini wrote:
> On Mon, 30 Aug 2010, Daniel De Graaf wrote:
>> On 08/30/2010 09:16 AM, Stefano Stabellini wrote:
>>> On Thu, 26 Aug 2010, Daniel De Graaf wrote:
>>>> When read_message returns -1 while a read is pending, an attempt is made
>>>> to lock h->request_mutex which is already locked by the reader. Change
>>>> the read thread to only exit on thread cancellation, where
>>>> read_thr_exists will return 0.
>>>>
>>>> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
>>>>
>>>> diff -r 6bebaf40e925 tools/xenstore/xs.c
>>>> --- a/tools/xenstore/xs.c  Tue Jul 20 13:42:17 2010 +0100
>>>> +++ b/tools/xenstore/xs.c  Thu Aug 26 17:08:55 2010 -0400
>>>> @@ -271,6 +271,7 @@
>>>>  {
>>>>  #ifdef USE_PTHREAD
>>>>    if (h->read_thr_exists) {
>>>> +          h->read_thr_exists = 0;
>>>>            pthread_cancel(h->read_thr);
>>>>            pthread_join(h->read_thr, NULL);
>>>>    }
>>>> @@ -667,7 +668,7 @@
>>>>    mutex_lock(&h->watch_mutex);
>>>>  
>>>>    /* Wait on the condition variable for a watch to fire. */
>>>> -  while (list_empty(&h->watch_list) && read_thread_exists(h))
>>>> +  while (list_empty(&h->watch_list) && (!read_from_thread || 
>>>> read_thread_exists(h)))
>>>>            condvar_wait(&h->watch_condvar, &h->watch_mutex, h);
>>>>    if (!read_thread_exists(h)) {
>>>>            mutex_unlock(&h->watch_mutex);
>>>
>>> read_from_thread is not declared in this function
>>>
>>
>> Sorry about that, looks like I picked the wrong patch to send. Corrected
>> patch attached.
>>
> 
> According to the description you gave, this patch fixes a problem
> occurring when read_message returns -1 while a read is pending in
> read_thread that is relevant only when USE_PTHREAD.
> Why are you changing a code path that is only compiled when
> !USE_PTHREAD?
> Also shouldn't you be checking the return value of read_message anyway?
> Considering that h->read_thr_exists is set to zero only on
> xs_daemon_close, and in that case pthread_cancel is called right after,
> I doubt that any of the operation run in read_thread after the loop will
> be executed.
> 

The value of read_message is not consistently checked in existing code.
I have submitted a separate patch that addresses this and also fixes the
deadlock, so neither of the patches in this thread are required.

-- 

Daniel De Graaf
National Security Agency

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>