|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH] libxl: check pending child signals on libxl__ev_child_inuse
Since we might call libxl__ev_child_inuse from the callback of another
event, and we might have pending signals from dead processes, update
the processes status before returning to the caller.
This was noticed because we processed POLLHUP before the child pipe,
so we ended up trying to kill the child process from the PULLHUP
callback, when the child was already dead. With this patch
libxl__ev_child_inuse returns false, and we no longer try to kill an
already dead child.
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxx>
---
tools/libxl/libxl_aoutils.c | 2 +-
tools/libxl/libxl_bootloader.c | 2 +-
tools/libxl/libxl_device.c | 6 ++--
tools/libxl/libxl_exec.c | 10 +++++++-
tools/libxl/libxl_fork.c | 42 ++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 9 ++++---
tools/libxl/libxl_save_callout.c | 4 +-
7 files changed, 62 insertions(+), 13 deletions(-)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 99972a2..9239657 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -336,7 +336,7 @@ int libxl__openptys(libxl__openpty_state *op,
out:
if (sockets[0] >= 0) close(sockets[0]);
libxl__carefd_close(for_child);
- if (libxl__ev_child_inuse(&op->child)) {
+ if (libxl__ev_child_inuse(gc, &op->child)) {
op->rc = rc;
/* we will get a callback when the child dies */
return 0;
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 7ba3502..50f91cc 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -286,7 +286,7 @@ static void bootloader_abort(libxl__egc *egc,
libxl__datacopier_kill(&bl->keystrokes);
libxl__datacopier_kill(&bl->display);
- if (libxl__ev_child_inuse(&bl->child)) {
+ if (libxl__ev_child_inuse(gc, &bl->child)) {
r = kill(bl->child.pid, SIGTERM);
if (r) LOGE(WARN, "after failure, failed to kill bootloader [%lu]",
(unsigned long)bl->child.pid);
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 91a6ef4..5e63d57 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -829,7 +829,7 @@ static void device_hotplug(libxl__egc *egc,
libxl__ao_device *aodev)
abort();
}
- assert(libxl__ev_child_inuse(&aodev->child));
+ assert(libxl__ev_child_inuse(gc, &aodev->child));
return;
@@ -845,7 +845,7 @@ static void device_hotplug_timeout_cb(libxl__egc *egc,
libxl__ev_time *ev,
libxl__ao_device *aodev = CONTAINER_OF(ev, *aodev, ev);
STATE_AO_GC(aodev->ao);
- assert(libxl__ev_child_inuse(&aodev->child));
+ assert(libxl__ev_child_inuse(gc, &aodev->child));
LOG(DEBUG, "killing hotplug script %s because of timeout", aodev->what);
if (kill(aodev->child.pid, SIGKILL)) {
LOGEV(ERROR, errno, "unable to kill hotplug script %s [%ld]",
@@ -915,7 +915,7 @@ static void device_hotplug_clean(libxl__gc *gc,
libxl__ao_device *aodev)
{
/* Clean events and check reentrancy */
libxl__ev_time_deregister(gc, &aodev->ev);
- assert(!libxl__ev_child_inuse(&aodev->child));
+ assert(!libxl__ev_child_inuse(gc, &aodev->child));
}
static void devices_remove_callback(libxl__egc *egc, libxl__ao_devices *aodevs,
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 0477386..ac32f24 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -157,6 +157,12 @@ out:
return rc ? SIGTERM : 0;
}
+int libxl__spawn_inuse(libxl__spawn_state *ss)
+{
+ STATE_AO_GC(ss->ao);
+ return libxl__ev_child_inuse(gc, &ss->mid);
+}
+
int libxl__wait_for_offspring(libxl__gc *gc,
uint32_t domid,
uint32_t timeout, char *what,
@@ -349,7 +355,7 @@ int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state
*ss)
static void spawn_cleanup(libxl__gc *gc, libxl__spawn_state *ss)
{
- assert(!libxl__ev_child_inuse(&ss->mid));
+ assert(!libxl__ev_child_inuse(gc, &ss->mid));
libxl__ev_time_deregister(gc, &ss->timeout);
libxl__ev_xswatch_deregister(gc, &ss->xswatch);
}
@@ -361,7 +367,7 @@ static void spawn_detach(libxl__gc *gc, libxl__spawn_state
*ss)
{
int r;
- assert(libxl__ev_child_inuse(&ss->mid));
+ assert(libxl__ev_child_inuse(gc, &ss->mid));
libxl__ev_time_deregister(gc, &ss->timeout);
libxl__ev_xswatch_deregister(gc, &ss->xswatch);
diff --git a/tools/libxl/libxl_fork.c b/tools/libxl/libxl_fork.c
index 044ddad..a0abc87 100644
--- a/tools/libxl/libxl_fork.c
+++ b/tools/libxl/libxl_fork.c
@@ -366,6 +366,48 @@ pid_t libxl__ev_child_fork(libxl__gc *gc, libxl__ev_child
*ch,
return rc;
}
+/*
+ * We need to update the child status, since there might be pending
+ * events on the child pipe.
+ */
+
+int libxl__ev_child_inuse(libxl__gc *gc, libxl__ev_child *childw_out)
+{
+ EGC_INIT_FROM_GC;
+ struct pollfd fds[1];
+ int selfpipe, rc;
+
+ CTX_LOCK;
+ selfpipe = libxl__fork_selfpipe_active(CTX);
+ if (selfpipe >= 0) {
+ fds[0].fd = selfpipe;
+ fds[0].events = POLLIN;
+
+ CTX_UNLOCK;
+ do {
+ rc = poll(fds, 1, 0);
+ } while (rc < 0 && errno == EINTR);
+ CTX_LOCK;
+
+ if (rc < 0) {
+ LIBXL__LOG_ERRNOVAL(CTX, LIBXL__LOG_ERROR, errno, "poll failed");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ if (rc) {
+ int e = libxl__self_pipe_eatall(selfpipe);
+ if (e) LIBXL__EVENT_DISASTER(egc, "read sigchld pipe", e, 0);
+ libxl__fork_selfpipe_woken(egc);
+ }
+ }
+ rc = childw_out->pid >= 0;
+
+out:
+ CTX_UNLOCK;
+ EGC_FREE;
+ return rc;
+}
+
void libxl_childproc_setmode(libxl_ctx *ctx, const libxl_childproc_hooks
*hooks,
void *user)
{
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 376a521..b9ce6eb 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -703,8 +703,7 @@ _hidden pid_t libxl__ev_child_fork(libxl__gc *gc,
libxl__ev_child *childw_out,
libxl__ev_child_callback *death);
static inline void libxl__ev_child_init(libxl__ev_child *childw_out)
{ childw_out->pid = -1; }
-static inline int libxl__ev_child_inuse(libxl__ev_child *childw_out)
- { return childw_out->pid >= 0; }
+_hidden int libxl__ev_child_inuse(libxl__gc *gc, libxl__ev_child *childw_out);
/* Useable (only) in the child to once more make the ctx useable for
* xenstore operations. logs failure in the form "what: <error
@@ -1136,8 +1135,7 @@ struct libxl__spawn_state {
libxl__ev_xswatch xswatch;
};
-static inline int libxl__spawn_inuse(libxl__spawn_state *ss)
- { return libxl__ev_child_inuse(&ss->mid); }
+_hidden int libxl__spawn_inuse(libxl__spawn_state *ss);
/*
* libxl_spawner_record_pid - Record given pid in xenstore
@@ -1528,6 +1526,9 @@ _hidden void libxl__egc_cleanup(libxl__egc *egc);
libxl__egc egc[1]; LIBXL_INIT_EGC(egc[0],ctx); \
EGC_GC
+#define EGC_INIT_FROM_GC \
+ libxl__egc egc[1]; LIBXL_INIT_EGC(egc[0],CTX);
+
#define EGC_FREE libxl__egc_cleanup(egc)
diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
index 078b7ee..685b417 100644
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -242,7 +242,7 @@ static void helper_failed(libxl__egc *egc,
libxl__save_helper_state *shs,
libxl__ev_fd_deregister(gc, &shs->readable);
- if (!libxl__ev_child_inuse(&shs->child)) {
+ if (!libxl__ev_child_inuse(gc, &shs->child)) {
helper_done(egc, shs);
return;
}
@@ -325,7 +325,7 @@ static void helper_done(libxl__egc *egc,
libxl__save_helper_state *shs)
libxl__ev_fd_deregister(gc, &shs->readable);
libxl__carefd_close(shs->pipes[0]); shs->pipes[0] = 0;
libxl__carefd_close(shs->pipes[1]); shs->pipes[1] = 0;
- assert(!libxl__ev_child_inuse(&shs->child));
+ assert(!libxl__ev_child_inuse(gc, &shs->child));
if (shs->toolstack_data_file) fclose(shs->toolstack_data_file);
shs->egc = egc;
--
1.7.7.5 (Apple Git-26)
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |