Re: [PATCH] libxl: check pending child signals on libxl__ev_child_inuse

[Roger Pau Monne <roger.pau@citrix.com>]

Roger Pau Monne wrote:
> Roger Pau Monne wrote:
>> 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.
>
> This is not correct, since we might assert that the child is running
> after a fork, but if the execution time is very short it might be dead.
>
> A better solution might be to change the order in which events are
> processed, the following patch makes libxl process child events before
> anything else.
>
> 8<--------------------------------------------------------------------
>
> [PATCH] libxl: check child signals before other events
>
> Check pending signals from child processes before processing other
> events, since the callbacks for other events might check the status of
> the child and try to kill them, when they might already be dead.

Since this is also prone to races, we should ignore POLLHUP on 
datacopier_readable and wait for the underlying process (the bootloader 
in this case) to exit. Linux probably doesn't send a POLLHUP when the 
process on the other end of the pty dies, but NetBSD does so, and since 
we process the fd events first we should ignore this and expect that the 
process dies.

8<---------------------------------------------------------------------

Subject: [PATCH] libxl: react correctly to POLLHUP

On datacopier_readable ignore PULLHUP, since we will shortly process
the death of the underlying process.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
  tools/libxl/libxl_aoutils.c |    8 ++++++++
  1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 99972a2..fa07154 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -102,6 +102,14 @@ static void datacopier_readable(libxl__egc *egc, 
libxl__ev_fd *ev,
      libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, toread);
      STATE_AO_GC(dc->ao);

+    if (revents & POLLHUP) {
+        LOG(DEBUG, "received POLLHUP on %s during copy of %s, "
+                   "waiting for process to die",
+                   dc->readwhat, dc->copywhat);
+        libxl__ev_fd_deregister(gc, &dc->toread);
+        return;
+    }
+
      if (revents & ~POLLIN) {
          LOG(ERROR, "unexpected poll event 0x%x (should be POLLIN)"
              " on %s during copy of %s", revents, dc->readwhat, 
dc->copywhat);
-- 
1.7.7.5 (Apple Git-26)