[Xen-changelog] [xen master] libxl: event system: Make libxl_sigchld_owner_libxl_always work

[patchbot@xxxxxxx]

commit 755aa9edb274f6f69a7afaffa399c70eb8205369
Author:     Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
AuthorDate: Tue Nov 5 16:43:59 2013 +0000
Commit:     Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CommitDate: Thu Nov 21 18:47:24 2013 +0000

    libxl: event system: Make libxl_sigchld_owner_libxl_always work
    
    Previously, libxl_sigchld_owner_libxl_always was mishandled.  It would
    result in libxl paying no attention to the sigchld self pipe.
    
    Fix this by fixing chldmode_ours so that it returns true iff we are
    supposed to be handling SIGCHLD.
    
    Additionally, we arrange to use chldmode_ours everywhere where we are
    installing/removing signal handlers and/or deciding whether to check
    the self pipe, etc.  This means it needs a new "creating" flag
    argument for the benefit of libxl__ev_child_fork, which needs to
    install the signal handler in libxl_sigchld_owner_libxl even if there
    are not currently any children.
    
    ctx->childproc_hooks->chldowner is now interpreted only by
    chldmode_ours.
    
    Reported-by: Bamvor Jian Zhang <bjzhang@xxxxxxxx>
    Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
    Cc: Bamvor Jian Zhang <bjzhang@xxxxxxxx>
    Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
    Cc: Jim Fehlig <jfehlig@xxxxxxxx>
    
    ---
    v2: Get sense of chldmode test right in perhaps_removehandler (!)
---
 tools/libxl/libxl_fork.c |   51 ++++++++++++++++++++++++---------------------
 1 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/tools/libxl/libxl_fork.c b/tools/libxl/libxl_fork.c
index ae6184c..a581763 100644
--- a/tools/libxl/libxl_fork.c
+++ b/tools/libxl/libxl_fork.c
@@ -224,18 +224,23 @@ int libxl__sigchld_installhandler(libxl__gc *gc) /* 
non-reentrant */
     return rc;
 }
 
-static int chldmode_ours(libxl_ctx *ctx)
+static bool chldmode_ours(libxl_ctx *ctx, bool creating)
 {
-    return ctx->childproc_hooks->chldowner == libxl_sigchld_owner_libxl;
+    switch (ctx->childproc_hooks->chldowner) {
+    case libxl_sigchld_owner_libxl:
+        return creating || !LIBXL_LIST_EMPTY(&ctx->children);
+    case libxl_sigchld_owner_mainloop:
+        return 0;
+    case libxl_sigchld_owner_libxl_always:
+        return 1;
+    }
+    abort();
 }
 
 int libxl__fork_selfpipe_active(libxl_ctx *ctx)
 {
     /* Returns the fd to read, or -1 */
-    if (!chldmode_ours(ctx))
-        return -1;
-
-    if (LIBXL_LIST_EMPTY(&ctx->children))
+    if (!chldmode_ours(ctx, 0))
         return -1;
 
     return ctx->sigchld_selfpipe[0];
@@ -243,11 +248,21 @@ int libxl__fork_selfpipe_active(libxl_ctx *ctx)
 
 static void perhaps_removehandler(libxl__gc *gc)
 {
-    if (LIBXL_LIST_EMPTY(&CTX->children) &&
-        CTX->childproc_hooks->chldowner != libxl_sigchld_owner_libxl_always)
+    if (!chldmode_ours(CTX, 0))
         libxl__sigchld_removehandler(gc);
 }
 
+static int perhaps_installhandler(libxl__gc *gc, bool creating)
+{
+    int rc;
+
+    if (chldmode_ours(CTX, creating)) {
+        rc = libxl__sigchld_installhandler(gc);
+        if (rc) return rc;
+    }
+    return 0;
+}
+
 static int childproc_reaped(libxl__egc *egc, pid_t pid, int status)
 {
     EGC_GC;
@@ -286,7 +301,7 @@ void libxl__fork_selfpipe_woken(libxl__egc *egc)
      * ctx must be locked EXACTLY ONCE */
     EGC_GC;
 
-    while (chldmode_ours(CTX) /* in case the app changes the mode */) {
+    while (chldmode_ours(CTX, 0) /* in case the app changes the mode */) {
         int status;
         pid_t pid = waitpid(-1, &status, WNOHANG);
 
@@ -332,10 +347,7 @@ pid_t libxl__ev_child_fork(libxl__gc *gc, libxl__ev_child 
*ch,
     CTX_LOCK;
     int rc;
 
-    if (chldmode_ours(CTX)) {
-        rc = libxl__sigchld_installhandler(gc);
-        if (rc) goto out;
-    }
+    perhaps_installhandler(gc, 1);
 
     pid_t pid =
         CTX->childproc_hooks->fork_replacement
@@ -382,17 +394,8 @@ void libxl_childproc_setmode(libxl_ctx *ctx, const 
libxl_childproc_hooks *hooks,
     ctx->childproc_hooks = hooks;
     ctx->childproc_user = user;
 
-    switch (ctx->childproc_hooks->chldowner) {
-    case libxl_sigchld_owner_mainloop:
-    case libxl_sigchld_owner_libxl:
-        libxl__sigchld_removehandler(gc);
-        break;
-    case libxl_sigchld_owner_libxl_always:
-        libxl__sigchld_installhandler(gc);
-        break;
-    default:
-        abort();
-    }
+    perhaps_removehandler(gc);
+    perhaps_installhandler(gc, 0); /* idempotent, ok to ignore errors for now 
*/
 
     CTX_UNLOCK;
     GC_FREE;
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog