[v2,7/7] t/helper/simple-ipc: convert test-simple-ipc to use start_bg_command

Message ID	6b7a058284b93fab52458b12a6aede5e8aed6652.1632152179.git.gitgitgadget@gmail.com (mailing list archive)
State	Accepted
Commit	05881a6fc9020b5c227a98490bf256d129da01f6
Headers	show Return-Path: <git-owner@kernel.org> Message-Id: <6b7a058284b93fab52458b12a6aede5e8aed6652.1632152179.git.gitgitgadget@gmail.com> In-Reply-To: <pull.1040.v2.git.1632152178.gitgitgadget@gmail.com> References: <pull.1040.git.1631738177.gitgitgadget@gmail.com> <pull.1040.v2.git.1632152178.gitgitgadget@gmail.com> Date: Mon, 20 Sep 2021 15:36:18 +0000 Subject: [PATCH v2 7/7] t/helper/simple-ipc: convert test-simple-ipc to use start_bg_command Fcc: Sent Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MIME-Version: 1.0 To: git@vger.kernel.org Cc: Eric Sunshine <sunshine@sunshineco.com>, Taylor Blau <me@ttaylorr.com>, =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason <avarab@gmail.com>, Jeff Hostetler <git@jeffhostetler.com>, Carlo Arenas <carenas@gmail.com>, Jeff Hostetler <jeffhost@microsoft.com>, Jeff Hostetler <jeffhost@microsoft.com> Precedence: bulk From: Jeff Hostetler <jeffhost@microsoft.com>
Series	Builtin FSMonitor Part 1 \| expand [v2,0/7] Builtin FSMonitor Part 1 [v2,1/7] trace2: add trace2_child_ready() to report on background children [v2,2/7] simple-ipc: preparations for supporting binary messages. [v2,3/7] simple-ipc: move definition of ipc_active_state outside of ifdef [v2,4/7] simple-ipc/ipc-win32: add trace2 debugging [v2,5/7] simple-ipc/ipc-win32: add Windows ACL to named pipe [v2,6/7] run-command: create start_bg_command [v2,7/7] t/helper/simple-ipc: convert test-simple-ipc to use start_bg_command

diff --git a/t/helper/test-simple-ipc.c b/t/helper/test-simple-ipc.c index 91345180750..28365ff85b6 100644 --- a/t/helper/test-simple-ipc.c +++ b/t/helper/test-simple-ipc.c @@ -9,6 +9,7 @@ #include "parse-options.h" #include "thread-utils.h" #include "strvec.h" +#include "run-command.h" #ifndef SUPPORTS_SIMPLE_IPC int cmd__simple_ipc(int argc, const char **argv) @@ -267,185 +268,71 @@ static int daemon__run_server(void) */ ret = ipc_server_run(cl_args.path, &opts, test_app_cb, (void*)&my_app_data); if (ret == -2) - error(_("socket/pipe already in use: '%s'"), cl_args.path); + error("socket/pipe already in use: '%s'", cl_args.path); else if (ret == -1) - error_errno(_("could not start server on: '%s'"), cl_args.path); + error_errno("could not start server on: '%s'", cl_args.path); return ret; } -#ifndef GIT_WINDOWS_NATIVE -/* - * This is adapted from `daemonize()`. Use `fork()` to directly create and - * run the daemon in a child process. - */ -static int spawn_server(pid_t *pid) -{ - struct ipc_server_opts opts = { - .nr_threads = cl_args.nr_threads, - }; +static start_bg_wait_cb bg_wait_cb; - *pid = fork(); - - switch (*pid) { - case 0: - if (setsid() == -1) - error_errno(_("setsid failed")); - close(0); - close(1); - close(2); - sanitize_stdfds(); +static int bg_wait_cb(const struct child_process *cp, void *cb_data) +{ + int s = ipc_get_active_state(cl_args.path); - return ipc_server_run(cl_args.path, &opts, test_app_cb, - (void*)&my_app_data); + switch (s) { + case IPC_STATE__LISTENING: + /* child is "ready" */ + return 0; - case -1: - return error_errno(_("could not spawn daemon in the background")); + case IPC_STATE__NOT_LISTENING: + case IPC_STATE__PATH_NOT_FOUND: + /* give child more time */ + return 1; default: - return 0; + case IPC_STATE__INVALID_PATH: + case IPC_STATE__OTHER_ERROR: + /* all the time in world won't help */ + return -1; } } -#else -/* - * Conceptually like `daemonize()` but different because Windows does not - * have `fork(2)`. Spawn a normal Windows child process but without the - * limitations of `start_command()` and `finish_command()`. - */ -static int spawn_server(pid_t *pid) -{ - char test_tool_exe[MAX_PATH]; - struct strvec args = STRVEC_INIT; - int in, out; - - GetModuleFileNameA(NULL, test_tool_exe, MAX_PATH); - - in = open("/dev/null", O_RDONLY); - out = open("/dev/null", O_WRONLY); - - strvec_push(&args, test_tool_exe); - strvec_push(&args, "simple-ipc"); - strvec_push(&args, "run-daemon"); - strvec_pushf(&args, "--name=%s", cl_args.path); - strvec_pushf(&args, "--threads=%d", cl_args.nr_threads); - - *pid = mingw_spawnvpe(args.v[0], args.v, NULL, NULL, in, out, out); - close(in); - close(out); - - strvec_clear(&args); - if (*pid < 0) - return error(_("could not spawn daemon in the background")); - - return 0; -} -#endif - -/* - * This is adapted from `wait_or_whine()`. Watch the child process and - * let it get started and begin listening for requests on the socket - * before reporting our success. - */ -static int wait_for_server_startup(pid_t pid_child) +static int daemon__start_server(void) { - int status; - pid_t pid_seen; - enum ipc_active_state s; - time_t time_limit, now; + struct child_process cp = CHILD_PROCESS_INIT; + enum start_bg_result sbgr; - time(&time_limit); - time_limit += cl_args.max_wait_sec; + strvec_push(&cp.args, "test-tool"); + strvec_push(&cp.args, "simple-ipc"); + strvec_push(&cp.args, "run-daemon"); + strvec_pushf(&cp.args, "--name=%s", cl_args.path); + strvec_pushf(&cp.args, "--threads=%d", cl_args.nr_threads); - for (;;) { - pid_seen = waitpid(pid_child, &status, WNOHANG); + cp.no_stdin = 1; + cp.no_stdout = 1; + cp.no_stderr = 1; - if (pid_seen == -1) - return error_errno(_("waitpid failed")); + sbgr = start_bg_command(&cp, bg_wait_cb, NULL, cl_args.max_wait_sec); - else if (pid_seen == 0) { - /* - * The child is still running (this should be - * the normal case). Try to connect to it on - * the socket and see if it is ready for - * business. - * - * If there is another daemon already running, - * our child will fail to start (possibly - * after a timeout on the lock), but we don't - * care (who responds) if the socket is live. - */ - s = ipc_get_active_state(cl_args.path); - if (s == IPC_STATE__LISTENING) - return 0; - - time(&now); - if (now > time_limit) - return error(_("daemon not online yet")); - - continue; - } + switch (sbgr) { + case SBGR_READY: + return 0; - else if (pid_seen == pid_child) { - /* - * The new child daemon process shutdown while - * it was starting up, so it is not listening - * on the socket. - * - * Try to ping the socket in the odd chance - * that another daemon started (or was already - * running) while our child was starting. - * - * Again, we don't care who services the socket. - */ - s = ipc_get_active_state(cl_args.path); - if (s == IPC_STATE__LISTENING) - return 0; + default: + case SBGR_ERROR: + case SBGR_CB_ERROR: + return error("daemon failed to start"); - /* - * We don't care about the WEXITSTATUS() nor - * any of the WIF*(status) values because - * `cmd__simple_ipc()` does the `!!result` - * trick on all function return values. - * - * So it is sufficient to just report the - * early shutdown as an error. - */ - return error(_("daemon failed to start")); - } + case SBGR_TIMEOUT: + return error("daemon not online yet"); - else - return error(_("waitpid is confused")); + case SBGR_DIED: + return error("daemon terminated"); } } -/* - * This process will start a simple-ipc server in a background process and - * wait for it to become ready. This is like `daemonize()` but gives us - * more control and better error reporting (and makes it easier to write - * unit tests). - */ -static int daemon__start_server(void) -{ - pid_t pid_child; - int ret; - - /* - * Run the actual daemon in a background process. - */ - ret = spawn_server(&pid_child); - if (pid_child <= 0) - return ret; - - /* - * Let the parent wait for the child process to get started - * and begin listening for requests on the socket. - */ - ret = wait_for_server_startup(pid_child); - - return ret; -} - /* * This process will run a quick probe to see if a simple-ipc server * is active on this path. @@ -548,7 +435,7 @@ static int client__stop_server(void) time(&now); if (now > time_limit) - return error(_("daemon has not shutdown yet")); + return error("daemon has not shutdown yet"); } }

[v2,7/7] t/helper/simple-ipc: convert test-simple-ipc to use start_bg_command

Commit Message

Comments

Patch