[Xen-devel] [XEN PATCH for-4.13 v3 1/7] libxl: Introduce libxl__ev_child_kill_deregister

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

Allow to deregister the callback associated with a child death event.

The death isn't immediate will need to be collected later, so the
ev_child machinery register its own callback.

libxl__ev_child_kill_deregister() might be called by an AO operation
that is finishing/cleaning up without a chance for libxl to be
notified of the child death (via SIGCHLD). So it is possible that the
application calls libxl_ctx_free() while there are still child around.
To avoid the application getting unexpected SIGCHLD, the libxl__ao
responsible for killing a child will have to wait until it has been
properly reaped.

Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---

Notes:
    v2:
    - Rename new fn to libxl__ev_child_kill_deregister
    - Rework documentation of the new API and if ev_child
    - Add debug log in libxl__ao_complete
    - Always call libxl_report_child_exitstatus() in child callback.

 tools/libxl/libxl_event.c    |  6 ++++-
 tools/libxl/libxl_fork.c     | 48 ++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h | 15 ++++++++---
 3 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 0370b6acdd1c..43155368de76 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1878,6 +1878,9 @@ void libxl__ao_complete(libxl__egc *egc, libxl__ao *ao, 
int rc)
     ao->complete = 1;
     ao->rc = rc;
     LIBXL_LIST_REMOVE(ao, inprogress_entry);
+    if (ao->outstanding_killed_child)
+        LOG(DEBUG, "ao %p: .. but waiting for %d fork to exit",
+            ao, ao->outstanding_killed_child);
     libxl__ao_complete_check_progress_reports(egc, ao);
 }
 
@@ -1891,7 +1894,8 @@ static bool ao_work_outstanding(libxl__ao *ao)
      * decrement progress_reports_outstanding, and call
      * libxl__ao_complete_check_progress_reports.
      */
-    return !ao->complete || ao->progress_reports_outstanding;
+    return !ao->complete || ao->progress_reports_outstanding
+        || ao->outstanding_killed_child;
 }
 
 void libxl__ao_complete_check_progress_reports(libxl__egc *egc, libxl__ao *ao)
diff --git a/tools/libxl/libxl_fork.c b/tools/libxl/libxl_fork.c
index eea3d5d4e68e..0f1b6b518c5c 100644
--- a/tools/libxl/libxl_fork.c
+++ b/tools/libxl/libxl_fork.c
@@ -678,6 +678,54 @@ int libxl__ev_child_xenstore_reopen(libxl__gc *gc, const 
char *what) {
     return rc;
 }
 
+typedef struct ev_child_killed {
+    libxl__ao *ao;
+    libxl__ev_child ch;
+} ev_child_killed;
+static void deregistered_child_callback(libxl__egc *, libxl__ev_child *,
+                                        pid_t, int status);
+
+void libxl__ev_child_kill_deregister(libxl__ao *ao, libxl__ev_child *ch,
+                                     int sig)
+{
+    AO_GC;
+
+    if (!libxl__ev_child_inuse(ch))
+        return;
+
+    pid_t pid = ch->pid;
+
+    ev_child_killed *new_ch = GCNEW(new_ch);
+    new_ch->ao = ao;
+    new_ch->ch.pid = pid;
+    new_ch->ch.callback = deregistered_child_callback;
+    LIBXL_LIST_INSERT_HEAD(&CTX->children, &new_ch->ch, entry);
+    ao->outstanding_killed_child++;
+
+    LIBXL_LIST_REMOVE(ch, entry);
+    ch->pid = -1;
+    int r = kill(pid, sig);
+    if (r)
+        LOGED(ERROR, ao->domid,
+              "failed to kill child [%ld] with signal %d",
+             (unsigned long)pid, sig);
+}
+
+static void deregistered_child_callback(libxl__egc *egc,
+                                        libxl__ev_child *ch,
+                                        pid_t pid,
+                                        int status)
+{
+    ev_child_killed *ck = CONTAINER_OF(ch, *ck, ch);
+    EGC_GC;
+
+    libxl_report_child_exitstatus(CTX, XTL_ERROR,
+                                  "killed fork (dying as expected)",
+                                  pid, status);
+    ck->ao->outstanding_killed_child--;
+    libxl__ao_complete_check_progress_reports(egc, ck->ao);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 6a614658c25d..4e433e110664 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -730,6 +730,7 @@ struct libxl__ao {
     libxl__poller *poller;
     uint32_t domid;
     LIBXL_TAILQ_ENTRY(libxl__ao) entry_for_callback;
+    int outstanding_killed_child;
 };
 
 #define LIBXL_INIT_GC(gc,ctx) do{               \
@@ -1155,9 +1156,14 @@ _hidden int libxl__ctx_evtchn_init(libxl__gc *gc); /* 
for libxl_ctx_alloc */
  * The parent may signal the child but it must not reap it.  That will
  * be done by the event machinery.
  *
- * It is not possible to "deregister" the child death event source.
- * It will generate exactly one event callback; until then the childw
- * is Active and may not be reused.
+ * The child death event will generate exactly one event callback; until
+ * then the childw is Active and may not be reused.
+ *
+ * libxl__ev_child_kill_deregister: Active -> Idle
+ *   This will transfer ownership of the child process death event from
+ *   `ch' to `ao', thus deregister the callback.
+ *   The `ao' completion will wait until the child have been reaped by the
+ *   event machinery.
  */
 _hidden pid_t libxl__ev_child_fork(libxl__gc *gc, libxl__ev_child *childw_out,
                                  libxl__ev_child_callback *death);
@@ -1165,6 +1171,9 @@ static inline void libxl__ev_child_init(libxl__ev_child 
*childw_out)
                 { childw_out->pid = -1; }
 static inline int libxl__ev_child_inuse(const libxl__ev_child *childw_out)
                 { return childw_out->pid >= 0; }
+_hidden void libxl__ev_child_kill_deregister(libxl__ao *ao,
+                                             libxl__ev_child *ch,
+                                             int sig);
 
 /* Useable (only) in the child to once more make the ctx useable for
  * xenstore operations.  logs failure in the form "what: <error
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel