An application which uses libxl_osevent_register_hooks, and doesn't
use libxl_sigchld_owner_mainloop, would never properly experience the
deaths of its (libxl) children.
This is because the SIGCHLD self pipe would be handled using ad-hoc
code in beforepoll_internal and afterpoll_internal. However, this is
no good if application is using the fd registration system instead; in
that case these functions would not be called and nothing would deal
with readability of the self pipe.
Fix this as follows:
The SIGCHLD self pipe now is now dealt with by a standard libxl__ev_fd
handler, which is registered and deregistered along with the SIGCHLD
handler itself. The handler function subsumes the ad-hoc response
code removed from beforepoll_internal, and the functionality of
libxl__fork_selfpipe_woken.
This is also tidier as the SIGCHLD self pipe is no longer touched
outside libxl_fork.c other than in ctx initialisation and teardown.
(The ad-hoc arrangements for poller->wakeup_pipe in
beforepoll_internal and afterpoll_internal are OK because the
libxl__poller mechanism exists to wake up threads which are sitting
inside libxl's poll loop, so is not applicable to the application's
event loop.)
Reported-by: Bamvor Jian Zhang <bjzhang@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Bamvor Jian Zhang <bjzhang@suse.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
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@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Bamvor Jian Zhang <bjzhang@suse.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
---
v2: Get sense of chldmode test right in perhaps_removehandler (!)
We are going to want the gc for libxl__ev_fd_register.
No functional change in this patch. Simply change the argument types,
and the actual arguments from ctx to gc. Inside these functions, use
CTX (the macro which uses gc) rather than the old formal parameter
ctx.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Without this, code interrupted by SIGCHLD may experience strange
values of errno. (As far as I know this is not the cause of any
reported bugs.)
This fix should be backported in due course.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
In 25182:6c3345d7e9d9 the practice of passing NULL to gc-using memory
allocation functions was introduced. However, the arrangements there
were not correct as committed, because the error handling and logging
depends on getting a ctx from the gc - so an allocation error would in
fact result in libxl dereferencing NULL.
Instead, provide a special dummy gc in the ctx, called `nogc_gc'. It
is marked out specially by having alloc_maxsize==-1, which is
otherwise invalid.
Functions which need to actually look into the gc use the new test
function gc_is_real (whose purpose is mainly clarity of the code) to
check whether the gc is the dummy one, and do nothing if it is. And
we provide a helper macro NOGC which uses the in-scope real gc to find
the ctx and hence the dummy gc (and which replaces the previous
#define NOGC NULL).
Change all callers which pass 0 or NULL to an allocation function to
use NOGC or &ctx->nogc_gc, as applicable in the context.
We add a comment near the definition of LIBXL_INIT_GC pointing out
that it isn't any more the only place a libxl__gc struct is
initialised, for the benefit of anyone changing the contents of gc's
in the future.
Also, actually document that libxl__ptr_add is legal with ptr==NULL,
and change a couple of calls not to check for NULL argument.
Reported-by: Bamvor Jian Zhang <bjzhang@suse.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Bamvor Jian Zhang <bjzhang@suse.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Committed-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Abolish libxl_fork. Its only callers were in xl. Its functionality
is now moved elsewhere, as follows:
* The "logging version of fork", which is what it was billed as, is now
xl_fork, which also dies on failure.
* Closing the xenstore handle in the child is now done in
libxl__ev_child_fork, which is the only remaining place where fork
is called in libxl.
* We provide a new function libxl__ev_child_xenstore_reopen for
in-libxl children to make the ctx useable for xenstore again.
* Consequently libxl__spawn_record_pid now no longer needs to mess
about with its own xenstore handle. As a bonus it can now just use
libxl__xs_write.
Also, since we have now converted all the forkers in libxl, we can
always honour the fork_replacement childproc hook - so do so.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
SIGCHLD or not; if it does, it has to tell us about deaths of our
children.
Currently there are no callers in libxl which use these facilities.
All code in libxl which forks needs to be converted and libxl_fork
needse to be be abolished.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
We introduce a new "carefd" concept, which relates to fds that we care
about not being inherited by long-lived children.
As yet we do not use this anywhere in libxl. Until all locations in
libxl which make such fds are converted, libxl__postfork may not work
entirely properly. If these locations do not use O_CLOEXEC (or use
calls for which there is no O_CLOEXEC) then multithreaded programs may
not work properly.
This introduces a new API call libxl_postfork_child_noexec which must
be called by applications which make long-running non-execing
children. Add the appropriate call to xl's postfork function.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Ian Jackson