Re: [Xen-devel] [PATCH] fix assert fail in libxl__sigchld_installhandler

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Bamvor Jian Zhang writes ("[PATCH] fix assert fail in 
libxl__sigchld_installhandler"):
> when libxl_sigchld_owner_libxl_always is used in libvirt or other
> tools stack, there is a assertion fail in restore in
> libxl__sigchld_installhandler during the following sequence:
> create -> save -> restore.
> 
> here is the backtrace:
>     #0  0x00007f7a522453d5 in raise () from /lib64/libc.so.6
>     #1  0x00007f7a52246858 in abort () from /lib64/libc.so.6
>     #2  0x00007f7a5223e2e2 in __assert_fail_base () from /lib64/libc.so.6
>     #3  0x00007f7a5223e392 in __assert_fail () from /lib64/libc.so.6
>     #4  0x00007f7a48008113 in libxl__sigchld_installhandler (
>         gc=gc@entry=0x7f7a4f080480) at libxl_fork.c:225
>     #5  0x00007f7a480081f4 in perhaps_installhandler 
> (gc=gc@entry=0x7f7a4f080480,
>         creating=creating@entry=false) at libxl_fork.c:272
>     #6  0x00007f7a4800851f in libxl_childproc_setmode (ctx=0x7f7a400c0ee0,
>         hooks=hooks@entry=0x7f7a482694a0 <childproc_hooks>,
>         user=user@entry=0x7f7a48474f80 <child_info>) at libxl_fork.c:427
> 
> the sigchld_owner exists but it is not the same as the new CTX.
> meanwhile the old sigchild is not removed in perhaps_removehandler.

I think the assertion has detected a bug in your use of libxl, not a
bug in libxl.

If you use libxl_sigchld_owner_libxl_always you're telling libxl that
this libxl_ctx should always own the entire application's SIGCHLDs.
That is obviously not compatible with having multiple different
libxl_ctx's.

If you are running multiple different long-running libxl operations in
parallel, then either:
 (a) they must all use the same libxl ctx
or
 (b) you must specify libxl_sigchld_owner_mainloop and demultiplex
     SIGCHLD yourself

This wasn't 100% clear from the doc comment.  I propose the patch
below to fix this.

Thanks,
Ian.

From 73b6484cbd5409101308c6c640eb7597f63256b9 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Date: Mon, 25 Nov 2013 11:53:28 +0000
Subject: [PATCH] libxl: doc comment: clarify SIGCHLD demultiplexing
 requirements

Update the comment to clarify that libxl_sigchld_owner_libxl_always
implies having only one libxl_ctx, and that
libxl_sigchld_owner_mainloop requires one call to
libxl_childproc_exited per ctx.

Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
---
 tools/libxl/libxl_event.h |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_event.h b/tools/libxl/libxl_event.h
index 27a65dc..5b2c689 100644
--- a/tools/libxl/libxl_event.h
+++ b/tools/libxl/libxl_event.h
@@ -446,13 +446,15 @@ void libxl_osevent_occurred_timeout(libxl_ctx *ctx, void 
*for_libxl)
  * 
  *     libxl_sigchld_owner_mainloop:
  *       The application must install a SIGCHLD handler and reap (at
- *       least) all of libxl's children and pass their exit status
- *       to libxl by calling libxl_childproc_exited.
+ *       least) all of libxl's children and pass their exit status to
+ *       libxl by calling libxl_childproc_exited.  (If the application
+ *       has multiple libxl ctx's, it must call libxl_childproc_exited
+ *       on each ctx.)
  *
  *     libxl_sigchld_owner_libxl_always:
  *       The application expects libxl to reap all of its children,
  *       and provides a callback to be notified of their exit
- *       statues.
+ *       statues.  The application must have only one libxl_ctx.
  *
  * An application which fails to call setmode, or which passes 0 for
  * hooks, while it uses any libxl operation which might
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel