# HG changeset patch
# User Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
# Date 1283348238 -3600
# Node ID 20e01ec42aabac99fa4ff2d5db630a04d4bc6820
# Parent eff592364826a7361b6d72adca596c5ee2331840
tools/xenstore: correctly handle errors from read_message
The return value of read_message needs to be checked in order to avoid
waiting forever for a message if there is an error on the communication
channel with xenstore. Currently, this is only checked if USE_PTHREAD is
defined (by checking for read thread exit), and that path is prone to
deadlock if request_mutex is held while waiting.
Since the failure of read_message leaves the socket in an undefined
state, close the socket and force all threads waiting on a read to return.
This also fixes xs_read_watch in the case where a read thread is not
running (in particular, this will happen if !USE_PTHREAD) by having it
read from the communication channel in the same way as read_reply.
Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
tools/xenstore/xs.c | 52 +++++++++++++++++++++++++++++++++-------------------
1 files changed, 33 insertions(+), 19 deletions(-)
diff -r eff592364826 -r 20e01ec42aab tools/xenstore/xs.c
--- a/tools/xenstore/xs.c Wed Sep 01 11:23:49 2010 +0100
+++ b/tools/xenstore/xs.c Wed Sep 01 14:37:18 2010 +0100
@@ -84,7 +84,7 @@ struct xs_handle {
#define mutex_lock(m) pthread_mutex_lock(m)
#define mutex_unlock(m) pthread_mutex_unlock(m)
#define condvar_signal(c) pthread_cond_signal(c)
-#define condvar_wait(c,m,hnd) pthread_cond_wait(c,m)
+#define condvar_wait(c,m) pthread_cond_wait(c,m)
#define cleanup_push(f, a) \
pthread_cleanup_push((void (*)(void *))(f), (void *)(a))
/*
@@ -111,7 +111,7 @@ struct xs_handle {
#define mutex_lock(m) ((void)0)
#define mutex_unlock(m) ((void)0)
#define condvar_signal(c) ((void)0)
-#define condvar_wait(c,m,hnd) read_message(hnd)
+#define condvar_wait(c,m) ((void)0)
#define cleanup_push(f, a) ((void)0)
#define cleanup_pop(run) ((void)0)
#define read_thread_exists(h) (0)
@@ -337,21 +337,20 @@ static void *read_reply(
read_from_thread = read_thread_exists(h);
-#ifdef USE_PTHREAD
/* Read from comms channel ourselves if there is no reader thread. */
if (!read_from_thread && (read_message(h) == -1))
return NULL;
+
+ mutex_lock(&h->reply_mutex);
+#ifdef USE_PTHREAD
+ while (list_empty(&h->reply_list) && read_from_thread && h->fd != -1)
+ condvar_wait(&h->reply_condvar, &h->reply_mutex);
#endif
-
- mutex_lock(&h->reply_mutex);
- while (list_empty(&h->reply_list) && (!read_from_thread ||
read_thread_exists(h)))
- condvar_wait(&h->reply_condvar, &h->reply_mutex, h);
- if (read_from_thread && !read_thread_exists(h)) {
+ if (list_empty(&h->reply_list)) {
mutex_unlock(&h->reply_mutex);
errno = EINVAL;
return NULL;
}
- assert(!list_empty(&h->reply_list));
msg = list_top(&h->reply_list, struct xs_stored_msg, list);
list_del(&msg->list);
assert(list_empty(&h->reply_list));
@@ -663,13 +662,22 @@ char **xs_read_watch(struct xs_handle *h
struct xs_stored_msg *msg;
char **ret, *strings, c = 0;
unsigned int num_strings, i;
+ int read_from_thread;
+
+ read_from_thread = read_thread_exists(h);
+
+ /* Read from comms channel ourselves if there is no reader thread. */
+ if (!read_from_thread && (read_message(h) == -1))
+ return NULL;
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))
- condvar_wait(&h->watch_condvar, &h->watch_mutex, h);
- if (!read_thread_exists(h)) {
+#ifdef USE_PTHREAD
+ while (list_empty(&h->watch_list) && read_from_thread && h->fd != -1)
+ condvar_wait(&h->watch_condvar, &h->watch_mutex);
+#endif
+ if (list_empty(&h->watch_list)) {
mutex_unlock(&h->watch_mutex);
errno = EINVAL;
return NULL;
@@ -965,21 +973,27 @@ static void *read_thread(void *arg)
static void *read_thread(void *arg)
{
struct xs_handle *h = arg;
+ int fd;
while (read_message(h) != -1)
continue;
- /* Kick anyone waiting for a reply */
- pthread_mutex_lock(&h->request_mutex);
- h->read_thr_exists = 0;
- pthread_mutex_unlock(&h->request_mutex);
-
+ /* An error return from read_message leaves the socket in an undefined
+ * state; we might have read only the header and not the message after
+ * it, or (more commonly) the other end has closed the connection.
+ * Since further communication is unsafe, close the socket.
+ */
+ fd = h->fd;
+ h->fd = -1;
+ close(fd);
+
+ /* wake up all waiters */
pthread_mutex_lock(&h->reply_mutex);
- pthread_cond_signal(&h->reply_condvar);
+ pthread_cond_broadcast(&h->reply_condvar);
pthread_mutex_unlock(&h->reply_mutex);
pthread_mutex_lock(&h->watch_mutex);
- pthread_cond_signal(&h->watch_condvar);
+ pthread_cond_broadcast(&h->watch_condvar);
pthread_mutex_unlock(&h->watch_mutex);
return NULL;
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
Home