[PATCH] tfork
Jeremy Allison
jra at samba.org
Tue Apr 18 23:22:33 UTC 2017
On Tue, Apr 18, 2017 at 09:55:56PM +0200, Ralph Böhme via samba-technical wrote:
> Hi!
>
> Attached is a patchset that adds a signal and waitpid() avoiding fork()
> replacement: tfork().
>
> It was based on an older patchset from metze, called double_fork at that time,
> that I extended to fork three-times in order to return a pollable status fd.
>
> It's intented use is in samba_runcmd_send() where it avoids calling waitpid()
> and dealing with SIGCHLD to get the exit status of the executed command.
>
> With this change it is safe to call samba_runcmd_send() in smbd code as an async
> replacement for smbrun().
>
> I have a usecase here where someone wants to run shell scripts from source3
> code without blocking.
>
> Please review & push if happy.
I'm looking at this right now, but it's making my brain hurt :-).
I've gotten as far as realizing the use of _exit() gets you out
of all sorts of reinit_after_fork() problems and atexit()
issues.
Still reviewing....
Jeremy.
> From 169a378fdf513202df410dcbec9ab14cb7e55340 Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <metze at samba.org>
> Date: Thu, 23 Sep 2010 18:10:02 +0200
> Subject: [PATCH 1/5] lib/util: add tfork()
>
> trippe-fork to avoid handling SIGCHLD in the parent.
>
> This function is a workaround for the problem of using fork() in
> library code. In that case the library should avoid to set a global
> signal handler for SIGCHLD, because the application may wants to use its
> own handler.
>
> status_fd can be used to wait for the child to exit and get its exit
> status.
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> lib/util/tfork.c | 329 +++++++++++++++++++++++++++++++++++++++++++++++++
> lib/util/tfork.h | 52 ++++++++
> lib/util/wscript_build | 4 +-
> 3 files changed, 383 insertions(+), 2 deletions(-)
> create mode 100644 lib/util/tfork.c
> create mode 100644 lib/util/tfork.h
>
> diff --git a/lib/util/tfork.c b/lib/util/tfork.c
> new file mode 100644
> index 0000000..23dba3f
> --- /dev/null
> +++ b/lib/util/tfork.c
> @@ -0,0 +1,329 @@
> +/*
> + fork on steroids to avoid SIGCHLD and waitpid
> +
> + Copyright (C) Stefan Metzmacher 2010
> + Copyright (C) Ralph Boehme 2017
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +#include "replace.h"
> +#include "system/wait.h"
> +#include "system/filesys.h"
> +#include "lib/util/samba_util.h"
> +#include "lib/util/sys_rw.h"
> +#include "lib/util/tfork.h"
> +#include "lib/util/debug.h"
> +
> +struct tfork_state {
> + void (*old_sig_chld)(int);
> + int status_pipe[2];
> + pid_t *parent;
> +
> + pid_t level0_pid;
> + int level0_status;
> +
> + pid_t level1_pid;
> + int level1_errno;
> +
> + pid_t level2_pid;
> + int level2_errno;
> +
> + pid_t level3_pid;
> +};
> +
> +/*
> + * TODO: We should make this global thread local
> + */
> +static struct tfork_state *tfork_global;
> +
> +static void tfork_sig_chld(int signum)
> +{
> + int ret;
> +
> + if (tfork_global->level1_pid > 0) {
> + ret = waitpid(tfork_global->level1_pid,
> + &tfork_global->level0_status,
> + WNOHANG);
> + if (ret == tfork_global->level1_pid) {
> + tfork_global->level1_pid = -1;
> + return;
> + }
> + }
> +
> + /*
> + * Not our child, forward to old handler
> + */
> +
> + if (tfork_global->old_sig_chld == SIG_IGN) {
> + return;
> + }
> +
> + if (tfork_global->old_sig_chld == SIG_DFL) {
> + return;
> + }
> +
> + tfork_global->old_sig_chld(signum);
> +}
> +
> +static pid_t level2_fork_and_wait(int child_ready_fd)
> +{
> + int ret;
> + int status;
> + ssize_t written;
> + pid_t pid;
> + int fd;
> + bool wait;
> +
> + /*
> + * Child level 2.
> + *
> + * Do a final fork and if the tfork() caller passed a status_fd, wait
> + * for child3 and return its exit status via status_fd.
> + */
> +
> + pid = fork();
> + if (pid == 0) {
> + /*
> + * Child level 3, this one finally returns from tfork() as child
> + * with pid 0.
> + *
> + * Cleanup all ressources we allocated before returning.
> + */
> + close(child_ready_fd);
> + close(tfork_global->status_pipe[1]);
> +
> + if (tfork_global->parent != NULL) {
> + /*
> + * we're in the child and return the level0 parent pid
> + */
> + *tfork_global->parent = tfork_global->level0_pid;
> + }
> +
> + anonymous_shared_free(tfork_global);
> + tfork_global = NULL;
> +
> + return 0;
> + }
> +
> + tfork_global->level3_pid = pid;
> + if (tfork_global->level3_pid == -1) {
> + tfork_global->level2_errno = errno;
> + _exit(0);
> + }
> +
> + sys_write(child_ready_fd, &(char){0}, 1);
> +
> + if (tfork_global->status_pipe[1] == -1) {
> + _exit(0);
> + }
> + wait = true;
> +
> + /*
> + * We're going to stay around until child3 exits, so lets close all fds
> + * other then the pipe fd we may have inherited from the caller.
> + */
> + fd = dup2(tfork_global->status_pipe[1], 0);
> + if (fd == -1) {
> + status = errno;
> + kill(tfork_global->level3_pid, SIGKILL);
> + wait = false;
> + }
> + closefrom(1);
> +
> + while (wait) {
> + ret = waitpid(tfork_global->level3_pid, &status, 0);
> + if (ret == -1) {
> + if (errno == EINTR) {
> + continue;
> + }
> + status = errno;
> + }
> + break;
> + }
> +
> + written = sys_write(fd, &status, sizeof(status));
> + if (written != sizeof(status)) {
> + abort();
> + }
> +
> + _exit(0);
> +}
> +
> +pid_t tfork(int *status_fd, pid_t *parent)
> +{
> + int ret;
> + pid_t pid;
> + pid_t child;
> +
> + tfork_global = (struct tfork_state *)
> + anonymous_shared_allocate(sizeof(struct tfork_state));
> + if (tfork_global == NULL) {
> + return -1;
> + }
> +
> + tfork_global->parent = parent;
> + tfork_global->status_pipe[0] = -1;
> + tfork_global->status_pipe[1] = -1;
> +
> + tfork_global->level0_pid = getpid();
> + tfork_global->level0_status = -1;
> + tfork_global->level1_pid = -1;
> + tfork_global->level1_errno = ECANCELED;
> + tfork_global->level2_pid = -1;
> + tfork_global->level2_errno = ECANCELED;
> + tfork_global->level3_pid = -1;
> +
> + if (status_fd != NULL) {
> + ret = pipe(&tfork_global->status_pipe[0]);
> + if (ret != 0) {
> + int saved_errno = errno;
> +
> + anonymous_shared_free(tfork_global);
> + tfork_global = NULL;
> + errno = saved_errno;
> + return -1;
> + }
> +
> + *status_fd = tfork_global->status_pipe[0];
> + }
> +
> + /*
> + * We need to set our own signal handler to prevent any existing signal
> + * handler from reaping our child.
> + */
> + tfork_global->old_sig_chld = CatchSignal(SIGCHLD, tfork_sig_chld);
> +
> + pid = fork();
> + if (pid == 0) {
> + int level2_pipe[2];
> + char c;
> + ssize_t nread;
> +
> + /*
> + * Child level 1.
> + *
> + * Restore SIGCHLD handler
> + */
> + CatchSignal(SIGCHLD, SIG_DFL);
> +
> + /*
> + * Close read end of the signal pipe, we don't need it anymore
> + * and don't want to leak it into childs.
> + */
> + if (tfork_global->status_pipe[0] != -1) {
> + close(tfork_global->status_pipe[0]);
> + tfork_global->status_pipe[0] = -1;
> + }
> +
> + /*
> + * Create a pipe for waiting for the child level 2 to finish
> + * forking.
> + */
> + ret = pipe(&level2_pipe[0]);
> + if (ret != 0) {
> + tfork_global->level1_errno = errno;
> + _exit(0);
> + }
> +
> + pid = fork();
> + if (pid == 0) {
> +
> + /*
> + * Child level 2.
> + */
> +
> + close(level2_pipe[0]);
> + return level2_fork_and_wait(level2_pipe[1]);
> + }
> +
> + tfork_global->level2_pid = pid;
> + if (tfork_global->level2_pid == -1) {
> + tfork_global->level1_errno = errno;
> + _exit(0);
> + }
> +
> + close(level2_pipe[1]);
> + level2_pipe[1] = -1;
> +
> + nread = sys_read(level2_pipe[0], &c, 1);
> + if (nread != 1) {
> + abort();
> + }
> + _exit(0);
> + }
> +
> + tfork_global->level1_pid = pid;
> + if (tfork_global->level1_pid == -1) {
> + int saved_errno = errno;
> +
> + anonymous_shared_free(tfork_global);
> + tfork_global = NULL;
> + errno = saved_errno;
> + return -1;
> + }
> +
> + /*
> + * By using the helper variable pid we avoid a TOCTOU with the signal
> + * handler that will set tfork_global->level1_pid to -1 (which would
> + * cause waitpid() to block waiting for another exitted child).
> + *
> + * We can't avoid the race waiting for pid twice (in the signal handler
> + * and then again here in the while loop), but we must avoid waiting for
> + * -1 and this does the trick.
> + */
> + pid = tfork_global->level1_pid;
> +
> + while (tfork_global->level1_pid != -1) {
> + ret = waitpid(pid, &tfork_global->level0_status, 0);
> + if (ret == -1 && errno == EINTR) {
> + continue;
> + }
> +
> + break;
> + }
> +
> + CatchSignal(SIGCHLD, tfork_global->old_sig_chld);
> +
> + if (tfork_global->level0_status != 0) {
> + anonymous_shared_free(tfork_global);
> + tfork_global = NULL;
> + errno = ECHILD;
> + return -1;
> + }
> +
> + if (tfork_global->level2_pid == -1) {
> + int saved_errno = tfork_global->level1_errno;
> +
> + anonymous_shared_free(tfork_global);
> + tfork_global = NULL;
> + errno = saved_errno;
> + return -1;
> + }
> +
> + if (tfork_global->level3_pid == -1) {
> + int saved_errno = tfork_global->level2_errno;
> +
> + anonymous_shared_free(tfork_global);
> + tfork_global = NULL;
> + errno = saved_errno;
> + return -1;
> + }
> +
> + child = tfork_global->level3_pid;
> + anonymous_shared_free(tfork_global);
> + tfork_global = NULL;
> +
> + return child;
> +}
> diff --git a/lib/util/tfork.h b/lib/util/tfork.h
> new file mode 100644
> index 0000000..0c62fc3
> --- /dev/null
> +++ b/lib/util/tfork.h
> @@ -0,0 +1,52 @@
> +/*
> + fork on steroids to avoid SIGCHLD and waitpid
> +
> + Copyright (C) Stefan Metzmacher 2010
> + Copyright (C) Ralph Boehme 2017
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +#ifndef LIB_UTIL_TFORK_H
> +#define LIB_UTIL_TFORK_H
> +
> +/**
> + * @brief a fork() that avoids SIGCHLD and waitpid
> + *
> + * This function is a workaround for the problem of using fork() in
> + * library code. In that case the library should avoid to set a global
> + * signal handler for SIGCHLD, because the application may wants to use its
> + * own handler.
> + *
> + * The child process will start with SIGCHLD handler set to SIG_DFL, so the
> + * child might need to setup its own handler.
> + *
> + * @param[out] status_fd If this is not NULL, tfork creates a pipe and returns
> + * the readable end via this pointer. The caller can
> + * wait for the process to finish by polling the
> + * status_fd for readability and can then read the exit
> + * status (an int).
> + *
> + * @param[out] parent The PID of the parent process, if 0 is returned
> + * otherwise the variable will not be touched at all.
> + * It is possible to pass NULL.
> + *
> + * @return On success, the PID of the child process is returned
> + * in the parent, and 0 is returned in the child. On
> + * failure, -1 is returned in the parent, no child
> + * process is created, and errno is set appropriately.
> + */
> +int tfork(int *status_fd, int *parent);
> +
> +#endif /* LIB_UTIL_TFORK_H */
> diff --git a/lib/util/wscript_build b/lib/util/wscript_build
> index 91505eb..a2fff9e 100644
> --- a/lib/util/wscript_build
> +++ b/lib/util/wscript_build
> @@ -120,10 +120,10 @@ else:
> idtree_random.c base64.c
> util_str.c util_str_common.c ms_fnmatch.c
> server_id.c dprintf.c pidfile.c
> - tevent_debug.c memcache.c unix_match.c''',
> + tevent_debug.c memcache.c unix_match.c tfork.c''',
> deps='samba-util-core DYNCONFIG close-low-fd tini tiniparser genrand',
> public_deps='talloc tevent execinfo pthread LIBCRYPTO charset util_setid systemd systemd-daemon',
> - public_headers='debug.h attr.h byteorder.h data_blob.h memory.h safe_string.h time.h talloc_stack.h string_wrappers.h idtree.h idtree_random.h blocking.h signal.h substitute.h fault.h genrand.h',
> + public_headers='debug.h attr.h byteorder.h data_blob.h memory.h safe_string.h time.h talloc_stack.h string_wrappers.h idtree.h idtree_random.h blocking.h signal.h substitute.h fault.h genrand.h tfork.h',
> header_path= [ ('dlinklist.h samba_util.h', '.'), ('*', 'util') ],
> local_include=False,
> vnum='0.0.1',
> --
> 2.9.3
>
>
> From e6d5438973b7cf588fd823e2b918f0a3bd326342 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 11 Apr 2017 17:32:01 +0200
> Subject: [PATCH 2/5] lib/util: add a test for tfork()
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> lib/util/tests/tfork.c | 102 ++++++++++++++++++++++++++++++++++++
> source4/torture/local/local.c | 1 +
> source4/torture/local/wscript_build | 1 +
> 3 files changed, 104 insertions(+)
> create mode 100644 lib/util/tests/tfork.c
>
> diff --git a/lib/util/tests/tfork.c b/lib/util/tests/tfork.c
> new file mode 100644
> index 0000000..7963aaf
> --- /dev/null
> +++ b/lib/util/tests/tfork.c
> @@ -0,0 +1,102 @@
> +/*
> + * Tests for tfork
> + *
> + * Copyright Ralph Boehme <slow at samba.org> 2017
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "replace.h"
> +#include <talloc.h>
> +#include "system/filesys.h"
> +#include "libcli/util/ntstatus.h"
> +#include "torture/torture.h"
> +#include "lib/util/data_blob.h"
> +#include "torture/local/proto.h"
> +#include "lib/util/tfork.h"
> +#include "lib/util/samba_util.h"
> +#include "lib/util/sys_rw.h"
> +
> +static bool test_tfork_simple(struct torture_context *tctx)
> +{
> + pid_t pid;
> + pid_t parent = getpid();
> + pid_t parent_arg;
> +
> + pid = tfork(NULL, &parent_arg);
> + if (pid == 0) {
> + torture_comment(tctx, "my parent pid is %d\n", parent);
> + torture_assert(tctx, parent == parent_arg, "tfork failed\n");
> + _exit(0);
> + }
> + if (pid == -1) {
> + torture_fail(tctx, "tfork failed\n");
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static bool test_tfork_status(struct torture_context *tctx)
> +{
> + pid_t child;
> + int status;
> + ssize_t nread;
> + int status_fd = -1;
> + bool ok = true;
> +
> + child = tfork(&status_fd, NULL);
> + if (child == 0) {
> + _exit(123);
> + }
> + if (child == -1) {
> + torture_fail(tctx, "tfork failed\n");
> + return false;
> + }
> +
> + nread = sys_read(status_fd, &status, sizeof(status));
> + if (nread != sizeof(status)) {
> + torture_fail(tctx, "sys_read failed\n");
> + }
> +
> + torture_assert_goto(tctx, WIFEXITED(status) == true, ok, done,
> + "tfork failed\n");
> + torture_assert_goto(tctx, WEXITSTATUS(status) == 123, ok, done,
> + "tfork failed\n");
> +
> + torture_comment(tctx, "exit status [%d]\n", WEXITSTATUS(status));
> +
> +done:
> + if (status_fd != -1) {
> + close(status_fd);
> + }
> +
> + return ok;
> +}
> +
> +struct torture_suite *torture_local_tfork(TALLOC_CTX *mem_ctx)
> +{
> + struct torture_suite *suite =
> + torture_suite_create(mem_ctx, "tfork");
> +
> + torture_suite_add_simple_test(suite,
> + "tfork_simple",
> + test_tfork_simple);
> +
> + torture_suite_add_simple_test(suite,
> + "tfork_status",
> + test_tfork_status);
> +
> + return suite;
> +}
> diff --git a/source4/torture/local/local.c b/source4/torture/local/local.c
> index 89066c5..353cc27 100644
> --- a/source4/torture/local/local.c
> +++ b/source4/torture/local/local.c
> @@ -75,6 +75,7 @@
> torture_local_nss,
> torture_local_fsrvp,
> torture_local_util_str_escape,
> + torture_local_tfork,
> NULL
> };
>
> diff --git a/source4/torture/local/wscript_build b/source4/torture/local/wscript_build
> index 2f1a7c8..6cbb14d 100644
> --- a/source4/torture/local/wscript_build
> +++ b/source4/torture/local/wscript_build
> @@ -21,6 +21,7 @@ TORTURE_LOCAL_SOURCE = '''../../../lib/util/charset/tests/iconv.c
> ../../../lib/util/tests/strv_util.c
> ../../../lib/util/tests/util.c
> ../../../lib/util/tests/util_str_escape.c
> + ../../../lib/util/tests/tfork.c
> verif_trailer.c
> nss_tests.c
> fsrvp_state.c'''
> --
> 2.9.3
>
>
> From a650b27bbfd0ffbb2fe812af2cbaa1f18f2e4375 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 11 Apr 2017 20:05:05 +0200
> Subject: [PATCH 3/5] lib/util: make use of tfork in samba_runcmd_send()
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> lib/util/util_runcmd.c | 103 +++++++++++++++++++++++++++----------------------
> lib/util/util_runcmd.h | 3 +-
> 2 files changed, 58 insertions(+), 48 deletions(-)
>
> diff --git a/lib/util/util_runcmd.c b/lib/util/util_runcmd.c
> index 9264cbb..7a2e1c6 100644
> --- a/lib/util/util_runcmd.c
> +++ b/lib/util/util_runcmd.c
> @@ -29,6 +29,8 @@
> #include "system/filesys.h"
> #include "../lib/util/tevent_unix.h"
> #include "../lib/util/util_runcmd.h"
> +#include "../lib/util/tfork.h"
> +#include "../lib/util/sys_rw.h"
>
> static int samba_runcmd_state_destructor(struct samba_runcmd_state *state)
> {
> @@ -106,7 +108,7 @@ struct tevent_req *samba_runcmd_send(TALLOC_CTX *mem_ctx,
> return tevent_req_post(req, ev);
> }
>
> - state->pid = fork();
> + state->pid = tfork(&state->fd_status, NULL);
> if (state->pid == (pid_t)-1) {
> close(p1[0]);
> close(p1[1]);
> @@ -130,10 +132,12 @@ struct tevent_req *samba_runcmd_send(TALLOC_CTX *mem_ctx,
> set_blocking(state->fd_stdout, false);
> set_blocking(state->fd_stderr, false);
> set_blocking(state->fd_stdin, false);
> + set_blocking(state->fd_status, false);
>
> smb_set_close_on_exec(state->fd_stdin);
> smb_set_close_on_exec(state->fd_stdout);
> smb_set_close_on_exec(state->fd_stderr);
> + smb_set_close_on_exec(state->fd_status);
>
> talloc_set_destructor(state, samba_runcmd_state_destructor);
>
> @@ -145,6 +149,7 @@ struct tevent_req *samba_runcmd_send(TALLOC_CTX *mem_ctx,
> if (tevent_req_nomem(state->fde_stdout, req)) {
> close(state->fd_stdout);
> close(state->fd_stderr);
> + close(state->fd_status);
> return tevent_req_post(req, ev);
> }
> tevent_fd_set_auto_close(state->fde_stdout);
> @@ -155,11 +160,26 @@ struct tevent_req *samba_runcmd_send(TALLOC_CTX *mem_ctx,
> samba_runcmd_io_handler,
> req);
> if (tevent_req_nomem(state->fde_stdout, req)) {
> + close(state->fd_stdout);
> close(state->fd_stderr);
> + close(state->fd_status);
> return tevent_req_post(req, ev);
> }
> tevent_fd_set_auto_close(state->fde_stderr);
>
> + state->fde_status = tevent_add_fd(ev, state,
> + state->fd_status,
> + TEVENT_FD_READ,
> + samba_runcmd_io_handler,
> + req);
> + if (tevent_req_nomem(state->fde_stdout, req)) {
> + close(state->fd_stdout);
> + close(state->fd_stderr);
> + close(state->fd_status);
> + return tevent_req_post(req, ev);
> + }
> + tevent_fd_set_auto_close(state->fde_status);
> +
> if (!timeval_is_zero(&endtime)) {
> tevent_req_set_endtime(req, ev, endtime);
> }
> @@ -231,6 +251,10 @@ static void samba_runcmd_io_handler(struct tevent_context *ev,
> char *p;
> int n, fd;
>
> + if (!(flags & TEVENT_FD_READ)) {
> + return;
> + }
> +
> if (fde == state->fde_stdout) {
> level = state->stdout_log_level;
> fd = state->fd_stdout;
> @@ -238,13 +262,40 @@ static void samba_runcmd_io_handler(struct tevent_context *ev,
> level = state->stderr_log_level;
> fd = state->fd_stderr;
> } else {
> - return;
> - }
> + int status;
> + ssize_t nread;
> +
> + /*
> + * Note: reading on a non-blocking pipe is guaranteed to deliver
> + * any pending data up to some kernel limit which is way beyond
> + * what we're reading here.
> + */
> + nread = sys_read(state->fd_status, &status, sizeof(status));
> + if (nread != sizeof(status)) {
> + DBG_ERR("Bad read on status pipe\n");
> + tevent_req_error(req, EIO);
> + return;
> + }
>
> - if (!(flags & TEVENT_FD_READ)) {
> + if (WIFEXITED(status)) {
> + status = WEXITSTATUS(status);
> + } else if (WIFSIGNALED(status)) {
> + status = WTERMSIG(status);
> + } else {
> + status = ECHILD;
> + }
> +
> + DBG_NOTICE("Child %s exited %d\n", state->arg0, status);
> + if (status != 0) {
> + tevent_req_error(req, status);
> + return;
> + }
> +
> + tevent_req_done(req);
> return;
> }
>
> +
> n = read(fd, &state->buf[state->buf_used],
> sizeof(state->buf) - state->buf_used);
> if (n > 0) {
> @@ -253,53 +304,11 @@ static void samba_runcmd_io_handler(struct tevent_context *ev,
> if (fde == state->fde_stdout) {
> talloc_free(fde);
> state->fde_stdout = NULL;
> + return;
> }
> if (fde == state->fde_stderr) {
> talloc_free(fde);
> state->fde_stderr = NULL;
> - }
> - if (state->fde_stdout == NULL &&
> - state->fde_stderr == NULL) {
> - int status;
> - /* the child has closed both stdout and
> - * stderr, assume its dead */
> - pid_t pid = waitpid(state->pid, &status, 0);
> - if (pid != state->pid) {
> - if (errno == ECHILD) {
> - /* this happens when the
> - parent has set SIGCHLD to
> - SIG_IGN. In that case we
> - can only get error
> - information for the child
> - via its logging. We should
> - stop using SIG_IGN on
> - SIGCHLD in the standard
> - process model.
> - */
> - DEBUG(0, ("Error in waitpid() unexpectedly got ECHILD "
> - "for %s child %d - %s, "
> - "someone has set SIGCHLD to SIG_IGN!\n",
> - state->arg0, (int)state->pid, strerror(errno)));
> - tevent_req_error(req, errno);
> - return;
> - }
> - DEBUG(0,("Error in waitpid() for child %s - %s \n",
> - state->arg0, strerror(errno)));
> - if (errno == 0) {
> - errno = ECHILD;
> - }
> - tevent_req_error(req, errno);
> - return;
> - }
> - status = WEXITSTATUS(status);
> - DEBUG(3,("Child %s exited with status %d\n",
> - state->arg0, status));
> - if (status != 0) {
> - tevent_req_error(req, status);
> - return;
> - }
> -
> - tevent_req_done(req);
> return;
> }
> return;
> diff --git a/lib/util/util_runcmd.h b/lib/util/util_runcmd.h
> index 26fdbc6..a887658 100644
> --- a/lib/util/util_runcmd.h
> +++ b/lib/util/util_runcmd.h
> @@ -27,7 +27,8 @@ struct samba_runcmd_state {
> int stderr_log_level;
> struct tevent_fd *fde_stdout;
> struct tevent_fd *fde_stderr;
> - int fd_stdin, fd_stdout, fd_stderr;
> + struct tevent_fd *fde_status;
> + int fd_stdin, fd_stdout, fd_stderr, fd_status;
> char *arg0;
> pid_t pid;
> char buf[1024];
> --
> 2.9.3
>
>
> From c4ae5a9a8b38c0bb0c745b73fd19cd9a868df306 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Sat, 15 Apr 2017 09:09:21 +0200
> Subject: [PATCH 4/5] wafsamba: add source directory define SRCDIR to config.h
>
> This will be used in the next commit to prepare the path to a test
> script in a smbtorture test.
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> buildtools/wafsamba/wscript | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/buildtools/wafsamba/wscript b/buildtools/wafsamba/wscript
> index 4eef008..430d164 100644
> --- a/buildtools/wafsamba/wscript
> +++ b/buildtools/wafsamba/wscript
> @@ -212,6 +212,8 @@ def configure(conf):
> conf.env.hlist = []
> conf.env.srcdir = conf.srcdir
>
> + conf.define('SRCDIR', conf.env['srcdir'])
> +
> if Options.options.timestamp_dependencies:
> conf.ENABLE_TIMESTAMP_DEPENDENCIES()
>
> --
> 2.9.3
>
>
> From 7219fbd0849b452eec155950df63db8f684e7029 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 11 Apr 2017 20:00:05 +0200
> Subject: [PATCH 5/5] lib/util: add a test for samba_runcmd_send()
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> lib/util/tests/tfork.c | 36 ++++++++++++++++++++++++++++++++++++
> testprogs/blackbox/tfork.sh | 15 +++++++++++++++
> 2 files changed, 51 insertions(+)
> create mode 100755 testprogs/blackbox/tfork.sh
>
> diff --git a/lib/util/tests/tfork.c b/lib/util/tests/tfork.c
> index 7963aaf..7a96928 100644
> --- a/lib/util/tests/tfork.c
> +++ b/lib/util/tests/tfork.c
> @@ -19,6 +19,7 @@
>
> #include "replace.h"
> #include <talloc.h>
> +#include <tevent.h>
> #include "system/filesys.h"
> #include "libcli/util/ntstatus.h"
> #include "torture/torture.h"
> @@ -85,6 +86,37 @@ done:
> return ok;
> }
>
> +static bool test_tfork_cmd_send(struct torture_context *tctx)
> +{
> + struct tevent_context *ev = NULL;
> + struct tevent_req *req = NULL;
> + const char *cmd[2] = { NULL, NULL };
> + bool ok = true;
> +
> + ev = tevent_context_init(tctx);
> + torture_assert_goto(tctx, ev != NULL, ok, done,
> + "tevent_context_init failed\n");
> +
> + cmd[0] = talloc_asprintf(tctx, "%s/testprogs/blackbox/tfork.sh", SRCDIR);
> + torture_assert_goto(tctx, cmd[0] != NULL, ok, done,
> + "talloc_asprintf failed\n");
> +
> + req = samba_runcmd_send(tctx, ev, timeval_zero(), 1, 0,
> + cmd, "foo", NULL);
> + torture_assert_goto(tctx, req != NULL, ok, done,
> + "samba_runcmd_send failed\n");
> +
> + ok = tevent_req_poll(req, ev);
> + torture_assert_goto(tctx, ok, ok, done, "tevent_req_poll failed\n");
> +
> + torture_comment(tctx, "samba_runcmd_send test finished\n");
> +
> +done:
> + TALLOC_FREE(ev);
> +
> + return ok;
> +}
> +
> struct torture_suite *torture_local_tfork(TALLOC_CTX *mem_ctx)
> {
> struct torture_suite *suite =
> @@ -98,5 +130,9 @@ struct torture_suite *torture_local_tfork(TALLOC_CTX *mem_ctx)
> "tfork_status",
> test_tfork_status);
>
> + torture_suite_add_simple_test(suite,
> + "tfork_cmd_send",
> + test_tfork_cmd_send);
> +
> return suite;
> }
> diff --git a/testprogs/blackbox/tfork.sh b/testprogs/blackbox/tfork.sh
> new file mode 100755
> index 0000000..0f75a8c
> --- /dev/null
> +++ b/testprogs/blackbox/tfork.sh
> @@ -0,0 +1,15 @@
> +#!/bin/sh
> +
> +sleep 1
> +
> +echo stdout >&1
> +echo $1 >&1
> +echo stderror >&2
> +
> +# close stdout and stderror, but don't exit yet
> +exec 1>&-
> +exec 2>&-
> +
> +sleep 1
> +
> +exit 0
> --
> 2.9.3
>
More information about the samba-technical
mailing list