From patchwork Mon Sep 20 15:36:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Hostetler X-Patchwork-Id: 12505705 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3E2B2C433FE for ; Mon, 20 Sep 2021 15:36:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1E2BB61107 for ; Mon, 20 Sep 2021 15:36:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237257AbhITPhv (ORCPT ); Mon, 20 Sep 2021 11:37:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39356 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236512AbhITPht (ORCPT ); Mon, 20 Sep 2021 11:37:49 -0400 Received: from mail-wr1-x42a.google.com (mail-wr1-x42a.google.com [IPv6:2a00:1450:4864:20::42a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 42798C061760 for ; Mon, 20 Sep 2021 08:36:22 -0700 (PDT) Received: by mail-wr1-x42a.google.com with SMTP id u15so30930351wru.6 for ; Mon, 20 Sep 2021 08:36:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=F7ftG0KmR42eaPTl8ZcevL+CQzKh8Tj9uSnnBfOlSG0=; b=egtEJ30qTUmv+FMkIvcC0ROGbgr04xDVIsrNsb/vTCjn2N2U1RfCLSc8pTdH9PO3uH xIlS/LdAWGIVhmWD5CS58danBdZs7ttyoIzLhzgo7G0AON4gcQ1di/ggTr3+V1E9xFrM 19/VQ61/aYqCFqZVKf9oYJmu7tXkrPXbf6SKssJLAKnl+Nh6Gin4MpCWrwKIsiHvIWWR vmY9gb1tPdRV1zdiEx4b/QAv7QwBy1QlnHYyYcs+CDVXoG6+AuZ+TBjXGC6Orups7jXx 5VKhgCxbwfrOpjW9u/PArGJCn3KYMl+VDcLARsy7PO38V0gpbCLBtXVvPfK64oSBwzFC I/aw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=F7ftG0KmR42eaPTl8ZcevL+CQzKh8Tj9uSnnBfOlSG0=; b=M/FAcuAY+BxQonKecr6IO+g+p7OeERpptQl4qNkkdPGurAebIIks6FGg81RJECb7G1 qzgxOrG2P2ERbRPzlyj5D+l8jRPb1RYN1N+6OEORO8WQR4m2uuEno1O3n9S4oMeGybrC 20hUMOIZDgdAEvtjeOUnZ/FuXx+LleZOToGcUGTC4CCtehTD2Wq8XEtrjDz1JflIPc8X e3SyS/2p2Lh8xBhMe7DH8Q+4cB6xLYCDpF/Y9ov4mHR5Pp+OeZwKgEgALVXh4fxUdbvt kOkV8jYpOR9b7Fnkx1Wjb/OzlRYB3W9g/Tx/iQQ8oyryJCaRO8IK7+kkUyeO+HNyT7po hzbw== X-Gm-Message-State: AOAM532T+intKC+Tf/fEnmYGQdse3tpcsA6LF3Hh4XY/lQ5FjZ/SbMhw LCHltJXTXtDtufbK1/TSAKo5oImwUxY= X-Google-Smtp-Source: ABdhPJxzVD/0FoS1m+UA78VCLML29xy1cxeKBRToxql6Jwgoh/n4JomkJnMcIVlm4/bpIpK6btEaEg== X-Received: by 2002:a5d:630a:: with SMTP id i10mr29850878wru.178.1632152180679; Mon, 20 Sep 2021 08:36:20 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id h8sm19165694wmb.35.2021.09.20.08.36.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Sep 2021 08:36:20 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Mon, 20 Sep 2021 15:36:12 +0000 Subject: [PATCH v2 1/7] trace2: add trace2_child_ready() to report on background children Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Eric Sunshine , Taylor Blau , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Jeff Hostetler , Carlo Arenas , Jeff Hostetler , Jeff Hostetler Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jeff Hostetler From: Jeff Hostetler Create "child_ready" event to capture the state of a child process created in the background. When a child command is started a "child_start" event is generated in the Trace2 log. For normal synchronous children, a "child_exit" event is later generated when the child exits or is terminated. The two events include information, such as the "child_id" and "pid", to allow post analysis to match-up the command line and exit status. When a child is started in the background (and may outlive the parent process), it is not possible for the parent to emit a "child_exit" event. Create a new "child_ready" event to indicate whether the child was successfully started. Also include the "child_id" and "pid" to allow similar post processing. This will be used in a later commit with the new "start_bg_command()". Signed-off-by: Jeff Hostetler --- Documentation/technical/api-trace2.txt | 40 ++++++++++++++++++++++++++ trace2.c | 31 ++++++++++++++++++++ trace2.h | 25 ++++++++++++++++ trace2/tr2_tgt.h | 5 ++++ trace2/tr2_tgt_event.c | 22 ++++++++++++++ trace2/tr2_tgt_normal.c | 14 +++++++++ trace2/tr2_tgt_perf.c | 15 ++++++++++ 7 files changed, 152 insertions(+) diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt index b9f3198fbe7..ef7fe02a8f7 100644 --- a/Documentation/technical/api-trace2.txt +++ b/Documentation/technical/api-trace2.txt @@ -613,6 +613,46 @@ stopping after the waitpid() and includes OS process creation overhead). So this time will be slightly larger than the atexit time reported by the child process itself. +`"child_ready"`:: + This event is generated after the current process has started + a background process and released all handles to it. ++ +------------ +{ + "event":"child_ready", + ... + "child_id":2, + "pid":14708, # child PID + "ready":"ready", # child ready state + "t_rel":0.110605 # observed run-time of child process +} +------------ ++ +Note that the session-id of the child process is not available to +the current/spawning process, so the child's PID is reported here as +a hint for post-processing. (But it is only a hint because the child +process may be a shell script which doesn't have a session-id.) ++ +This event is generated after the child is started in the background +and given a little time to boot up and start working. If the child +startups normally and while the parent is still waiting, the "ready" +field will have the value "ready". +If the child is too slow to start and the parent times out, the field +will have the value "timeout". +If the child starts but the parent is unable to probe it, the field +will have the value "error". ++ +After the parent process emits this event, it will release all of its +handles to the child process and treat the child as a background +daemon. So even if the child does eventually finish booting up, +the parent will not emit an updated event. ++ +Note that the `t_rel` field contains the observed run time in seconds +when the parent released the child process into the background. +The child is assumed to be a long-running daemon process and may +outlive the parent process. So the parent's child event times should +not be compared to the child's atexit times. + `"exec"`:: This event is generated before git attempts to `exec()` another command rather than starting a child process. diff --git a/trace2.c b/trace2.c index b9b154ac440..b2d471526fd 100644 --- a/trace2.c +++ b/trace2.c @@ -394,6 +394,37 @@ void trace2_child_exit_fl(const char *file, int line, struct child_process *cmd, us_elapsed_child); } +void trace2_child_ready_fl(const char *file, int line, + struct child_process *cmd, + const char *ready) +{ + struct tr2_tgt *tgt_j; + int j; + uint64_t us_now; + uint64_t us_elapsed_absolute; + uint64_t us_elapsed_child; + + if (!trace2_enabled) + return; + + us_now = getnanotime() / 1000; + us_elapsed_absolute = tr2tls_absolute_elapsed(us_now); + + if (cmd->trace2_child_us_start) + us_elapsed_child = us_now - cmd->trace2_child_us_start; + else + us_elapsed_child = 0; + + for_each_wanted_builtin (j, tgt_j) + if (tgt_j->pfn_child_ready_fl) + tgt_j->pfn_child_ready_fl(file, line, + us_elapsed_absolute, + cmd->trace2_child_id, + cmd->pid, + ready, + us_elapsed_child); +} + int trace2_exec_fl(const char *file, int line, const char *exe, const char **argv) { diff --git a/trace2.h b/trace2.h index 9b7286c572f..6fe9802598b 100644 --- a/trace2.h +++ b/trace2.h @@ -253,6 +253,31 @@ void trace2_child_exit_fl(const char *file, int line, struct child_process *cmd, #define trace2_child_exit(cmd, code) \ trace2_child_exit_fl(__FILE__, __LINE__, (cmd), (code)) +/** + * Emits a "child_ready" message containing the "child-id" and a flag + * indicating whether the child was considered "ready" when we + * released it. + * + * This function should be called after starting a daemon process in + * the background (and after giving it sufficient time to boot + * up) to indicate that we no longer control or own it. + * + * The "ready" argument should contain one of { "ready", "timeout", + * "error" } to indicate the state of the running daemon when we + * released it. + * + * If the daemon process fails to start or it exits or is terminated + * while we are still waiting for it, the caller should emit a + * regular "child_exit" to report the normal process exit information. + * + */ +void trace2_child_ready_fl(const char *file, int line, + struct child_process *cmd, + const char *ready); + +#define trace2_child_ready(cmd, ready) \ + trace2_child_ready_fl(__FILE__, __LINE__, (cmd), (ready)) + /** * Emit an 'exec' event prior to calling one of exec(), execv(), * execvp(), and etc. On Unix-derived systems, this will be the diff --git a/trace2/tr2_tgt.h b/trace2/tr2_tgt.h index 1f66fd65730..65f94e15748 100644 --- a/trace2/tr2_tgt.h +++ b/trace2/tr2_tgt.h @@ -45,6 +45,10 @@ typedef void(tr2_tgt_evt_child_exit_fl_t)(const char *file, int line, uint64_t us_elapsed_absolute, int cid, int pid, int code, uint64_t us_elapsed_child); +typedef void(tr2_tgt_evt_child_ready_fl_t)(const char *file, int line, + uint64_t us_elapsed_absolute, + int cid, int pid, const char *ready, + uint64_t us_elapsed_child); typedef void(tr2_tgt_evt_thread_start_fl_t)(const char *file, int line, uint64_t us_elapsed_absolute); @@ -116,6 +120,7 @@ struct tr2_tgt { tr2_tgt_evt_alias_fl_t *pfn_alias_fl; tr2_tgt_evt_child_start_fl_t *pfn_child_start_fl; tr2_tgt_evt_child_exit_fl_t *pfn_child_exit_fl; + tr2_tgt_evt_child_ready_fl_t *pfn_child_ready_fl; tr2_tgt_evt_thread_start_fl_t *pfn_thread_start_fl; tr2_tgt_evt_thread_exit_fl_t *pfn_thread_exit_fl; tr2_tgt_evt_exec_fl_t *pfn_exec_fl; diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c index 578a9a5287a..70cfc2f77cc 100644 --- a/trace2/tr2_tgt_event.c +++ b/trace2/tr2_tgt_event.c @@ -383,6 +383,27 @@ static void fn_child_exit_fl(const char *file, int line, jw_release(&jw); } +static void fn_child_ready_fl(const char *file, int line, + uint64_t us_elapsed_absolute, int cid, int pid, + const char *ready, uint64_t us_elapsed_child) +{ + const char *event_name = "child_ready"; + struct json_writer jw = JSON_WRITER_INIT; + double t_rel = (double)us_elapsed_child / 1000000.0; + + jw_object_begin(&jw, 0); + event_fmt_prepare(event_name, file, line, NULL, &jw); + jw_object_intmax(&jw, "child_id", cid); + jw_object_intmax(&jw, "pid", pid); + jw_object_string(&jw, "ready", ready); + jw_object_double(&jw, "t_rel", 6, t_rel); + jw_end(&jw); + + tr2_dst_write_line(&tr2dst_event, &jw.json); + + jw_release(&jw); +} + static void fn_thread_start_fl(const char *file, int line, uint64_t us_elapsed_absolute) { @@ -610,6 +631,7 @@ struct tr2_tgt tr2_tgt_event = { fn_alias_fl, fn_child_start_fl, fn_child_exit_fl, + fn_child_ready_fl, fn_thread_start_fl, fn_thread_exit_fl, fn_exec_fl, diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c index a5751c88644..58d9e430f05 100644 --- a/trace2/tr2_tgt_normal.c +++ b/trace2/tr2_tgt_normal.c @@ -251,6 +251,19 @@ static void fn_child_exit_fl(const char *file, int line, strbuf_release(&buf_payload); } +static void fn_child_ready_fl(const char *file, int line, + uint64_t us_elapsed_absolute, int cid, int pid, + const char *ready, uint64_t us_elapsed_child) +{ + struct strbuf buf_payload = STRBUF_INIT; + double elapsed = (double)us_elapsed_child / 1000000.0; + + strbuf_addf(&buf_payload, "child_ready[%d] pid:%d ready:%s elapsed:%.6f", + cid, pid, ready, elapsed); + normal_io_write_fl(file, line, &buf_payload); + strbuf_release(&buf_payload); +} + static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute, int exec_id, const char *exe, const char **argv) { @@ -330,6 +343,7 @@ struct tr2_tgt tr2_tgt_normal = { fn_alias_fl, fn_child_start_fl, fn_child_exit_fl, + fn_child_ready_fl, NULL, /* thread_start */ NULL, /* thread_exit */ fn_exec_fl, diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c index af4d65a0a5f..e4acca13d64 100644 --- a/trace2/tr2_tgt_perf.c +++ b/trace2/tr2_tgt_perf.c @@ -360,6 +360,20 @@ static void fn_child_exit_fl(const char *file, int line, strbuf_release(&buf_payload); } +static void fn_child_ready_fl(const char *file, int line, + uint64_t us_elapsed_absolute, int cid, int pid, + const char *ready, uint64_t us_elapsed_child) +{ + const char *event_name = "child_ready"; + struct strbuf buf_payload = STRBUF_INIT; + + strbuf_addf(&buf_payload, "[ch%d] pid:%d ready:%s", cid, pid, ready); + + perf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute, + &us_elapsed_child, NULL, &buf_payload); + strbuf_release(&buf_payload); +} + static void fn_thread_start_fl(const char *file, int line, uint64_t us_elapsed_absolute) { @@ -553,6 +567,7 @@ struct tr2_tgt tr2_tgt_perf = { fn_alias_fl, fn_child_start_fl, fn_child_exit_fl, + fn_child_ready_fl, fn_thread_start_fl, fn_thread_exit_fl, fn_exec_fl, From patchwork Mon Sep 20 15:36:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Hostetler X-Patchwork-Id: 12505707 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D3A62C433F5 for ; Mon, 20 Sep 2021 15:36:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B53F661159 for ; Mon, 20 Sep 2021 15:36:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237723AbhITPhx (ORCPT ); Mon, 20 Sep 2021 11:37:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39358 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237047AbhITPhu (ORCPT ); Mon, 20 Sep 2021 11:37:50 -0400 Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E81EAC061574 for ; Mon, 20 Sep 2021 08:36:22 -0700 (PDT) Received: by mail-wr1-x431.google.com with SMTP id u18so29178296wrg.5 for ; Mon, 20 Sep 2021 08:36:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=icLaEkrW27nB6n3VpTAiOzUFJuxu99LJ0DDiPaBiKuk=; b=Btl/HQblQ5wkXAHFnpwzXcH3jhmbnbL9GFeJuBHhISbC2YMrQPxpWrId6wgweboQMa 1lPRORSV5dNNs8UXoOuzJxU746ALiqNLOpEQfPwo0rL3hz266QDoM3etz4rFRRr37Y6b u1yCWiQxOh30rac+GvafaYflYMmvbRmKnhw9ufBNHyDOaet2XF16OJDQ2vMRQ09xIY/T bwaQ6EOw6muDD0e0xGjXdsCDBUq4VXH7yh8ReHfllYKz1wNyuPNXtSKnkIJ5lxMQZhMi jHdcHb4yCnVFwzgD5ZaBLTOirCckpG+F8cbUL0j7ZKWq/+R7zr9qiopKtSL9pPe+r/SX wM4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=icLaEkrW27nB6n3VpTAiOzUFJuxu99LJ0DDiPaBiKuk=; b=8HPIUky+cAmBsD3bbx+iBZRcvJPlDPqdqGcNBwpbYlEZkqkZr7uw+cVegmU2+VeyWA z6Zegbj37ml0q0A2qKPTmXVIN3eLmAwJ53ZTmqVTd0osCXs5X3+uN2bWHwhE/5a3djgx s38O6OAXnX96oGWOumfb/PxzLaoTjF8QYH7cOEyAO0/K4HUqCfwykHOuS905Pwa7JbR3 2UHkvH9s3Ua+RwykLx6tLR6Dq+esC1KN+xDCfVfSWgwMNuXHi1PDNB91bBuXCiDgb0my iLIMd8ZmKoE50w1AfywEQQAAPUYSvM/oRUk2T22fGdsDB7WBvwMf195eVssxkN4oLi75 MFdg== X-Gm-Message-State: AOAM533u4ePQqcRyQREHuq+8bvZyZzOigI+bnj4azHNn3vi+sr62pVC3 y4ZuxdEu503nJbtlAptBMSr45a/wLY0= X-Google-Smtp-Source: ABdhPJxcZrYnEaazDaiO+jXvIUoY2dzTykLP4cJmmci+X9k/1YZ+/1W/pIFu8Gdbgz2Gmrs3w2v+MQ== X-Received: by 2002:a05:600c:4f42:: with SMTP id m2mr23380850wmq.151.1632152181509; Mon, 20 Sep 2021 08:36:21 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id o1sm2520307wmq.26.2021.09.20.08.36.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Sep 2021 08:36:21 -0700 (PDT) Message-Id: <258baa0df8ca8c068fd953bd52cbe246d2e7fa09.1632152178.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Mon, 20 Sep 2021 15:36:13 +0000 Subject: [PATCH v2 2/7] simple-ipc: preparations for supporting binary messages. Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Eric Sunshine , Taylor Blau , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Jeff Hostetler , Carlo Arenas , Jeff Hostetler , Jeff Hostetler Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jeff Hostetler From: Jeff Hostetler Add `command_len` argument to the Simple IPC API. In my original Simple IPC API, I assumed that the request would always be a null-terminated string of text characters. The `command` argument was just a `const char *`. I found a caller that would like to pass a binary command to the daemon, so I am amending the Simple IPC API to receive `const char *command, size_t command_len` arguments. I considered changing the `command` argument to be a `void *`, but the IPC layer simply passes it to the pkt-line layer which takes a `const char *`, so to avoid confusion I left it as is. Note, the response side has always been a `struct strbuf` which includes the buffer and length, so we already support returning a binary answer. (Yes, it feels a little weird returning a binary buffer in a `strbuf`, but it works.) Signed-off-by: Jeff Hostetler --- compat/simple-ipc/ipc-unix-socket.c | 14 +++++++----- compat/simple-ipc/ipc-win32.c | 14 +++++++----- simple-ipc.h | 7 ++++-- t/helper/test-simple-ipc.c | 34 +++++++++++++++++++---------- 4 files changed, 46 insertions(+), 23 deletions(-) diff --git a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c index 1927e6ef4bc..4e28857a0a1 100644 --- a/compat/simple-ipc/ipc-unix-socket.c +++ b/compat/simple-ipc/ipc-unix-socket.c @@ -168,7 +168,8 @@ void ipc_client_close_connection(struct ipc_client_connection *connection) int ipc_client_send_command_to_connection( struct ipc_client_connection *connection, - const char *message, struct strbuf *answer) + const char *message, size_t message_len, + struct strbuf *answer) { int ret = 0; @@ -176,7 +177,7 @@ int ipc_client_send_command_to_connection( trace2_region_enter("ipc-client", "send-command", NULL); - if (write_packetized_from_buf_no_flush(message, strlen(message), + if (write_packetized_from_buf_no_flush(message, message_len, connection->fd) < 0 || packet_flush_gently(connection->fd) < 0) { ret = error(_("could not send IPC command")); @@ -197,7 +198,8 @@ done: int ipc_client_send_command(const char *path, const struct ipc_client_connect_options *options, - const char *message, struct strbuf *answer) + const char *message, size_t message_len, + struct strbuf *answer) { int ret = -1; enum ipc_active_state state; @@ -208,7 +210,9 @@ int ipc_client_send_command(const char *path, if (state != IPC_STATE__LISTENING) return ret; - ret = ipc_client_send_command_to_connection(connection, message, answer); + ret = ipc_client_send_command_to_connection(connection, + message, message_len, + answer); ipc_client_close_connection(connection); @@ -503,7 +507,7 @@ static int worker_thread__do_io( if (ret >= 0) { ret = worker_thread_data->server_data->application_cb( worker_thread_data->server_data->application_data, - buf.buf, do_io_reply_callback, &reply_data); + buf.buf, buf.len, do_io_reply_callback, &reply_data); packet_flush_gently(reply_data.fd); } diff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c index 8dc7bda087d..8e889d6a506 100644 --- a/compat/simple-ipc/ipc-win32.c +++ b/compat/simple-ipc/ipc-win32.c @@ -208,7 +208,8 @@ void ipc_client_close_connection(struct ipc_client_connection *connection) int ipc_client_send_command_to_connection( struct ipc_client_connection *connection, - const char *message, struct strbuf *answer) + const char *message, size_t message_len, + struct strbuf *answer) { int ret = 0; @@ -216,7 +217,7 @@ int ipc_client_send_command_to_connection( trace2_region_enter("ipc-client", "send-command", NULL); - if (write_packetized_from_buf_no_flush(message, strlen(message), + if (write_packetized_from_buf_no_flush(message, message_len, connection->fd) < 0 || packet_flush_gently(connection->fd) < 0) { ret = error(_("could not send IPC command")); @@ -239,7 +240,8 @@ done: int ipc_client_send_command(const char *path, const struct ipc_client_connect_options *options, - const char *message, struct strbuf *response) + const char *message, size_t message_len, + struct strbuf *response) { int ret = -1; enum ipc_active_state state; @@ -250,7 +252,9 @@ int ipc_client_send_command(const char *path, if (state != IPC_STATE__LISTENING) return ret; - ret = ipc_client_send_command_to_connection(connection, message, response); + ret = ipc_client_send_command_to_connection(connection, + message, message_len, + response); ipc_client_close_connection(connection); @@ -458,7 +462,7 @@ static int do_io(struct ipc_server_thread_data *server_thread_data) if (ret >= 0) { ret = server_thread_data->server_data->application_cb( server_thread_data->server_data->application_data, - buf.buf, do_io_reply_callback, &reply_data); + buf.buf, buf.len, do_io_reply_callback, &reply_data); packet_flush_gently(reply_data.fd); diff --git a/simple-ipc.h b/simple-ipc.h index 2c48a5ee004..9c7330fcda0 100644 --- a/simple-ipc.h +++ b/simple-ipc.h @@ -107,7 +107,8 @@ void ipc_client_close_connection(struct ipc_client_connection *connection); */ int ipc_client_send_command_to_connection( struct ipc_client_connection *connection, - const char *message, struct strbuf *answer); + const char *message, size_t message_len, + struct strbuf *answer); /* * Used by the client to synchronously connect and send and receive a @@ -119,7 +120,8 @@ int ipc_client_send_command_to_connection( */ int ipc_client_send_command(const char *path, const struct ipc_client_connect_options *options, - const char *message, struct strbuf *answer); + const char *message, size_t message_len, + struct strbuf *answer); /* * Simple IPC Server Side API. @@ -144,6 +146,7 @@ typedef int (ipc_server_reply_cb)(struct ipc_server_reply_data *, */ typedef int (ipc_server_application_cb)(void *application_data, const char *request, + size_t request_len, ipc_server_reply_cb *reply_cb, struct ipc_server_reply_data *reply_data); diff --git a/t/helper/test-simple-ipc.c b/t/helper/test-simple-ipc.c index 42040ef81b1..91345180750 100644 --- a/t/helper/test-simple-ipc.c +++ b/t/helper/test-simple-ipc.c @@ -112,7 +112,7 @@ static int app__slow_command(ipc_server_reply_cb *reply_cb, /* * The client sent a command followed by a (possibly very) large buffer. */ -static int app__sendbytes_command(const char *received, +static int app__sendbytes_command(const char *received, size_t received_len, ipc_server_reply_cb *reply_cb, struct ipc_server_reply_data *reply_data) { @@ -123,6 +123,13 @@ static int app__sendbytes_command(const char *received, int errs = 0; int ret; + /* + * The test is setup to send: + * "sendbytes" SP + */ + if (received_len < strlen("sendbytes ")) + BUG("received_len is short in app__sendbytes_command"); + if (skip_prefix(received, "sendbytes ", &p)) len_ballast = strlen(p); @@ -160,7 +167,7 @@ static ipc_server_application_cb test_app_cb; * by this application. */ static int test_app_cb(void *application_data, - const char *command, + const char *command, size_t command_len, ipc_server_reply_cb *reply_cb, struct ipc_server_reply_data *reply_data) { @@ -173,7 +180,7 @@ static int test_app_cb(void *application_data, if (application_data != (void*)&my_app_data) BUG("application_cb: application_data pointer wrong"); - if (!strcmp(command, "quit")) { + if (command_len == 4 && !strncmp(command, "quit", 4)) { /* * The client sent a "quit" command. This is an async * request for the server to shutdown. @@ -193,22 +200,23 @@ static int test_app_cb(void *application_data, return SIMPLE_IPC_QUIT; } - if (!strcmp(command, "ping")) { + if (command_len == 4 && !strncmp(command, "ping", 4)) { const char *answer = "pong"; return reply_cb(reply_data, answer, strlen(answer)); } - if (!strcmp(command, "big")) + if (command_len == 3 && !strncmp(command, "big", 3)) return app__big_command(reply_cb, reply_data); - if (!strcmp(command, "chunk")) + if (command_len == 5 && !strncmp(command, "chunk", 5)) return app__chunk_command(reply_cb, reply_data); - if (!strcmp(command, "slow")) + if (command_len == 4 && !strncmp(command, "slow", 4)) return app__slow_command(reply_cb, reply_data); - if (starts_with(command, "sendbytes ")) - return app__sendbytes_command(command, reply_cb, reply_data); + if (command_len >= 10 && starts_with(command, "sendbytes ")) + return app__sendbytes_command(command, command_len, + reply_cb, reply_data); return app__unhandled_command(command, reply_cb, reply_data); } @@ -488,7 +496,9 @@ static int client__send_ipc(void) options.wait_if_busy = 1; options.wait_if_not_found = 0; - if (!ipc_client_send_command(cl_args.path, &options, command, &buf)) { + if (!ipc_client_send_command(cl_args.path, &options, + command, strlen(command), + &buf)) { if (buf.len) { printf("%s\n", buf.buf); fflush(stdout); @@ -556,7 +566,9 @@ static int do_sendbytes(int bytecount, char byte, const char *path, strbuf_addstr(&buf_send, "sendbytes "); strbuf_addchars(&buf_send, byte, bytecount); - if (!ipc_client_send_command(path, options, buf_send.buf, &buf_resp)) { + if (!ipc_client_send_command(path, options, + buf_send.buf, buf_send.len, + &buf_resp)) { strbuf_rtrim(&buf_resp); printf("sent:%c%08d %s\n", byte, bytecount, buf_resp.buf); fflush(stdout); From patchwork Mon Sep 20 15:36:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jeff Hostetler X-Patchwork-Id: 12505709 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5AFC5C433EF for ; Mon, 20 Sep 2021 15:36:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4087661159 for ; Mon, 20 Sep 2021 15:36:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237962AbhITPhz (ORCPT ); Mon, 20 Sep 2021 11:37:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39362 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231953AbhITPhu (ORCPT ); Mon, 20 Sep 2021 11:37:50 -0400 Received: from mail-wr1-x433.google.com (mail-wr1-x433.google.com [IPv6:2a00:1450:4864:20::433]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 72671C061760 for ; Mon, 20 Sep 2021 08:36:23 -0700 (PDT) Received: by mail-wr1-x433.google.com with SMTP id q11so30913202wrr.9 for ; Mon, 20 Sep 2021 08:36:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:mime-version :content-transfer-encoding:fcc:to:cc; bh=WhOw3Obe0gOgQO+AfU1jSOzrMAyzSWPdmF+5plxu8Aw=; b=V7M3jefCrOs6tgX3le0Ro9Db+jIPJGTtwCDQeC1g348MEF5/hpVo0WJdaS5BYa+ZWY KdAqSRJ5PTcQxf02u3vdeWO82p8oe9poh5VrrBSVh04iPPXjF1hThmu3gdms5QkdaQ0+ UEO6VHFlPotowcFTHRHmLdj8TAMKl05B7BEvpSvjmwjzAOUFYc2JEII9u24LBSmF5vcl K9N7JDpoB8AfNjCyUKw8dmQ0rkxRUh5xLllbBtHG4nDBbCFAOZJVvN26jmqPaRjgGt+h 4ZSaSG+lZ6VGriFXO6/ibdsq5EflCFTeSgdvZRHWhLu0O1BalrKuVeuGduZZ6908PzQe X/6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:mime-version:content-transfer-encoding:fcc:to:cc; bh=WhOw3Obe0gOgQO+AfU1jSOzrMAyzSWPdmF+5plxu8Aw=; b=NhcgEoAfFyPLPrxsiypTd61XiWo0JQUmpHzT6/W3j0O7pFVySN2sf2K1gRTkC+Xzdd O0MRyNreScN1K3D+PNsYyo2Hgcv0BwmF0YFWd6pB2d59+FETXGcFYk63yYGoQP+/GgXU fW3KkFf9VkwIQRDg8rH+oopA+HNnSS7OcoT9WMotpQRKRGIq9MNEM08AR+MnAzbzkkeS XE64SI9gy3e+XHFbb9/g1R3sZ6KqmdWzj4apBgNekzdon7/KW93uZFwGu7QUH6z4o4IJ /EhTvf1H2UjDyZ8oGYsDCps1NaM/WvzYT9KYgIRNDh0fhuAVqvqJJYdyrPwdiIzqDjMy Mmbw== X-Gm-Message-State: AOAM533iedoF1utWBITeMjuDT5nwU7VTdvJCFBve/oYY1fYswUymD8FF T+15UGZIpcjO0/rDKFyULwiSZl5xndA= X-Google-Smtp-Source: ABdhPJwPhGruEBttD9XJ5KbCTukwB4plZsjXNHPNzinEN9UTS+f+a3v/gORcw+ba3kBazNFven9qeQ== X-Received: by 2002:a5d:47ad:: with SMTP id 13mr4883238wrb.77.1632152182112; Mon, 20 Sep 2021 08:36:22 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id o17sm16414526wrj.96.2021.09.20.08.36.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Sep 2021 08:36:21 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Mon, 20 Sep 2021 15:36:14 +0000 Subject: [PATCH v2 3/7] simple-ipc: move definition of ipc_active_state outside of ifdef MIME-Version: 1.0 Fcc: Sent To: git@vger.kernel.org Cc: Eric Sunshine , Taylor Blau , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Jeff Hostetler , Carlo Arenas , Jeff Hostetler , Jeff Hostetler Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jeff Hostetler From: Jeff Hostetler From: Carlo Marcelo Arenas Belón Move the declartion of the `enum ipc_active_state` type outside of the SUPPORTS_SIMPLE_IPC ifdef. A later commit will introduce the `fsmonitor_ipc__*()` API and stub in a "mock" implementation that requires this enum in some function signatures. Signed-off-by: Carlo Marcelo Arenas Belón Signed-off-by: Jeff Hostetler --- simple-ipc.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/simple-ipc.h b/simple-ipc.h index 9c7330fcda0..b396293bdfc 100644 --- a/simple-ipc.h +++ b/simple-ipc.h @@ -5,13 +5,6 @@ * See Documentation/technical/api-simple-ipc.txt */ -#ifdef SUPPORTS_SIMPLE_IPC -#include "pkt-line.h" - -/* - * Simple IPC Client Side API. - */ - enum ipc_active_state { /* * The pipe/socket exists and the daemon is waiting for connections. @@ -43,6 +36,13 @@ enum ipc_active_state { IPC_STATE__OTHER_ERROR, }; +#ifdef SUPPORTS_SIMPLE_IPC +#include "pkt-line.h" + +/* + * Simple IPC Client Side API. + */ + struct ipc_client_connect_options { /* * Spin under timeout if the server is running but can't From patchwork Mon Sep 20 15:36:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Hostetler X-Patchwork-Id: 12505711 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D988DC433F5 for ; Mon, 20 Sep 2021 15:36:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C30B86112D for ; Mon, 20 Sep 2021 15:36:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238101AbhITPh6 (ORCPT ); Mon, 20 Sep 2021 11:37:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39364 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237252AbhITPhv (ORCPT ); Mon, 20 Sep 2021 11:37:51 -0400 Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 170D9C061574 for ; Mon, 20 Sep 2021 08:36:24 -0700 (PDT) Received: by mail-wr1-x435.google.com with SMTP id u15so30930621wru.6 for ; Mon, 20 Sep 2021 08:36:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=JxZ883T7/40ehnE1C9Wcit7uGo6rCuzaGwJ2rCX+cMk=; b=SytR7/CDpvGxwYs5jad53bn1ugslIIU+zblG5Iy/K1JT8Mm6hik7+sMbxVL4ssnBnD 5DE6ZirpVxNgn+z7MveMlpGr325pxUHEwxAIOBjVSvCapWNT2YVN4rCWzO00pXNjd371 /L7QdOcg96RKmaG52uKyP6GoWwgbpIzoM5X87Dwk/X1bduUI17TqlYBPkaaOL6PDHfjk D146xMNfa+a8fnFriGKuJ88liFd026mb23bhMqqevzBM2AFfM4cBqNVdicM7feLwp3gE ZPrwpiKLlqNonl4CMclK4E9/U4tWdIu5M3F+gj9MMRPzQFoOZxQyiV0TM/MDD5/fHZZz Og/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=JxZ883T7/40ehnE1C9Wcit7uGo6rCuzaGwJ2rCX+cMk=; b=Yx4dLvBTIHPGukN8Kgr/oBPQEbTIOaaIClVsaV3i2jgdQHuiTi9laKm29HkpkYk+qG 7YJ/PlIXOhMvbH4emRQwFYtfZUc7mtNRCiYKQe1ud4YCsXriqqaz+IDyBVSNvnyTlq3/ DTFFUIxnWmu0g290r+KDGILIQzLtEJsbDtsqTP+vXHiMoiZ5gy0srKOErRQwBmAG2un7 cfsyH10TOm9h9/+i+/vn37d5hCHqhiPt/rKLt2AWoi0Okuk9lRguhlIuKNj/AXPA2A+h RYVr8je4gs6fSdfPy860/HmaGQF/rNiTv4N4waMoecWWovZ2Q3GOwu0MIwaeHRbXzdBT N4cQ== X-Gm-Message-State: AOAM530ImuRWOq8aSvyOGb5utDS+2tU/bXV/RmsTGG4Us+osM9nsLmqw AOTIHdKvzq02hXC6Dvzjw65sP0/6sQY= X-Google-Smtp-Source: ABdhPJwHoUXU8gmyq0KrF7vE00woLlsIHmltKKS+1WjabXjhj1iTqldACfvglsb+CsOI/XNBw/Mt+w== X-Received: by 2002:a5d:6c67:: with SMTP id r7mr13048769wrz.29.1632152182773; Mon, 20 Sep 2021 08:36:22 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id d9sm20584235wrb.36.2021.09.20.08.36.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Sep 2021 08:36:22 -0700 (PDT) Message-Id: <82b6ce0dd6a8934c202dfd38f89daf81e0ed45c7.1632152178.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Mon, 20 Sep 2021 15:36:15 +0000 Subject: [PATCH v2 4/7] simple-ipc/ipc-win32: add trace2 debugging Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Eric Sunshine , Taylor Blau , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Jeff Hostetler , Carlo Arenas , Jeff Hostetler , Jeff Hostetler Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jeff Hostetler From: Jeff Hostetler Create "ipc-debug" category events to log unexpected errors when creating Simple-IPC connections. Signed-off-by: Jeff Hostetler --- compat/simple-ipc/ipc-win32.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c index 8e889d6a506..a8dd46bd922 100644 --- a/compat/simple-ipc/ipc-win32.c +++ b/compat/simple-ipc/ipc-win32.c @@ -49,6 +49,9 @@ static enum ipc_active_state get_active_state(wchar_t *pipe_path) if (GetLastError() == ERROR_FILE_NOT_FOUND) return IPC_STATE__PATH_NOT_FOUND; + trace2_data_intmax("ipc-debug", NULL, "getstate/waitpipe/gle", + (intmax_t)GetLastError()); + return IPC_STATE__OTHER_ERROR; } @@ -109,9 +112,15 @@ static enum ipc_active_state connect_to_server( t_start_ms = (DWORD)(getnanotime() / 1000000); if (!WaitNamedPipeW(wpath, timeout_ms)) { - if (GetLastError() == ERROR_SEM_TIMEOUT) + DWORD gleWait = GetLastError(); + + if (gleWait == ERROR_SEM_TIMEOUT) return IPC_STATE__NOT_LISTENING; + trace2_data_intmax("ipc-debug", NULL, + "connect/waitpipe/gle", + (intmax_t)gleWait); + return IPC_STATE__OTHER_ERROR; } @@ -133,17 +142,31 @@ static enum ipc_active_state connect_to_server( break; /* try again */ default: + trace2_data_intmax("ipc-debug", NULL, + "connect/createfile/gle", + (intmax_t)gle); + return IPC_STATE__OTHER_ERROR; } } if (!SetNamedPipeHandleState(hPipe, &mode, NULL, NULL)) { + gle = GetLastError(); + trace2_data_intmax("ipc-debug", NULL, + "connect/setpipestate/gle", + (intmax_t)gle); + CloseHandle(hPipe); return IPC_STATE__OTHER_ERROR; } *pfd = _open_osfhandle((intptr_t)hPipe, O_RDWR|O_BINARY); if (*pfd < 0) { + gle = GetLastError(); + trace2_data_intmax("ipc-debug", NULL, + "connect/openosfhandle/gle", + (intmax_t)gle); + CloseHandle(hPipe); return IPC_STATE__OTHER_ERROR; } From patchwork Mon Sep 20 15:36:16 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Hostetler X-Patchwork-Id: 12505715 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E9FC4C433F5 for ; Mon, 20 Sep 2021 15:36:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D37EA61107 for ; Mon, 20 Sep 2021 15:36:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238148AbhITPiA (ORCPT ); Mon, 20 Sep 2021 11:38:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39370 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237282AbhITPhv (ORCPT ); Mon, 20 Sep 2021 11:37:51 -0400 Received: from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com [IPv6:2a00:1450:4864:20::42b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D75BBC061760 for ; Mon, 20 Sep 2021 08:36:24 -0700 (PDT) Received: by mail-wr1-x42b.google.com with SMTP id i23so30941822wrb.2 for ; Mon, 20 Sep 2021 08:36:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=0Cl1yv7LeDWfZGdfZCfdqCe5HaL5g/B5qt/Hs8wqPpE=; b=kWYmE7g14+DiyNf8kmpx6GLpEQ/ecXEiSvEGb/QFW0Kn9xH1jKQsovtmfZA2Us7a6N XDSdPVkF/kaR1NSqFFYgoVZ4DQlqBfEmb5/nPO5Do02cPp5zfYJdimaz6ghKzuY6UzUz sxR1FOdnH6KPsBA/OfiMWkuCHybEakmb8EuwAj1NUcRGQJ5g504wsSutqADqAndGENl4 WYx5NqtcDrNZ7/TcgBcKgn/ZkQpPr4qFMwGk4IYwpBiNJhgTYV65Jd6Lt2BSLsb0MEoa HZepQouarBaSwTu60A32vo9TUeDp8V3jb0DM/heJxzfCDny9C3v3SsPNAQM3eALMH81+ 8Ytw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=0Cl1yv7LeDWfZGdfZCfdqCe5HaL5g/B5qt/Hs8wqPpE=; b=F74utdod/vaVXOA3ozczkdWfPrMLunceDMcXcQ45ESmCfBqvOfgYdSuOch2wtaDCrE XOxEWe5wfI3bUGDC0vymtolwhbjVRfwnyw/eW+GWJDnPMsEbX1iWYmREeOlMYfkxJTLO 48tUMFYVU6aysfNh44d6GVjgz+wYkJHXysMTZ0UfOOGwtecFqXAejvyku99duO7RNd1E ILSsDc6CmV/OQbAz1UCT/IOnju+DD1F0pOtumbXEhhRiF4amGO3wkk53qnPdqvnpS6uk WSSit4wfJDQCb/UH18wov/VQhVBZi9BABdYp7+ZOtPGyHqMG0wzDK+w2zXGjZ8jRNhNS yPHA== X-Gm-Message-State: AOAM531WqJsHF+uDXgH1etiXmDcWGaeUx7xzoJ1rRYohj752i3a8Rc+2 drNTl+AL1b8ao4Wc+XMLH807FWjZS8s= X-Google-Smtp-Source: ABdhPJwv/W6uZxbKQUhNINNC15ODlawHre/TbSS5GJ+jO61k5drNesMV6RHyerw4c9M/ZDRAUjp+Bg== X-Received: by 2002:a5d:6750:: with SMTP id l16mr28859553wrw.174.1632152183430; Mon, 20 Sep 2021 08:36:23 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id g1sm466312wmk.2.2021.09.20.08.36.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Sep 2021 08:36:23 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Mon, 20 Sep 2021 15:36:16 +0000 Subject: [PATCH v2 5/7] simple-ipc/ipc-win32: add Windows ACL to named pipe Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Eric Sunshine , Taylor Blau , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Jeff Hostetler , Carlo Arenas , Jeff Hostetler , Jeff Hostetler Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jeff Hostetler From: Jeff Hostetler Set an ACL on the named pipe to allow the well-known group EVERYONE to read and write to the IPC server's named pipe. In the event that the daemon was started with elevation, allow non-elevated clients to communicate with the daemon. Signed-off-by: Jeff Hostetler --- compat/simple-ipc/ipc-win32.c | 140 +++++++++++++++++++++++++++++++--- 1 file changed, 129 insertions(+), 11 deletions(-) diff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c index a8dd46bd922..20ea7b65e0b 100644 --- a/compat/simple-ipc/ipc-win32.c +++ b/compat/simple-ipc/ipc-win32.c @@ -3,6 +3,8 @@ #include "strbuf.h" #include "pkt-line.h" #include "thread-utils.h" +#include "accctrl.h" +#include "aclapi.h" #ifndef SUPPORTS_SIMPLE_IPC /* @@ -592,11 +594,132 @@ finished: return NULL; } +/* + * We need to build a Windows "SECURITY_ATTRIBUTES" object and use it + * to apply an ACL when we create the initial instance of the Named + * Pipe. The construction is somewhat involved and consists of + * several sequential steps and intermediate objects. + * + * We use this structure to hold these intermediate pointers so that + * we can free them as a group. (It is unclear from the docs whether + * some of these intermediate pointers can be freed before we are + * finished using the "lpSA" member.) + */ +struct my_sa_data +{ + PSID pEveryoneSID; + PACL pACL; + PSECURITY_DESCRIPTOR pSD; + LPSECURITY_ATTRIBUTES lpSA; +}; + +static void init_sa(struct my_sa_data *d) +{ + memset(d, 0, sizeof(*d)); +} + +static void release_sa(struct my_sa_data *d) +{ + if (d->pEveryoneSID) + FreeSid(d->pEveryoneSID); + if (d->pACL) + LocalFree(d->pACL); + if (d->pSD) + LocalFree(d->pSD); + if (d->lpSA) + LocalFree(d->lpSA); + + memset(d, 0, sizeof(*d)); +} + +/* + * Create SECURITY_ATTRIBUTES to apply to the initial named pipe. The + * creator of the first server instance gets to set the ACLs on it. + * + * We allow the well-known group `EVERYONE` to have read+write access + * to the named pipe so that clients can send queries to the daemon + * and receive the response. + * + * Normally, this is not necessary since the daemon is usually + * automatically started by a foreground command like `git status`, + * but in those cases where an elevated Git command started the daemon + * (such that the daemon itself runs with elevation), we need to add + * the ACL so that non-elevated commands can write to it. + * + * The following document was helpful: + * https://docs.microsoft.com/en-us/windows/win32/secauthz/creating-a-security-descriptor-for-a-new-object-in-c-- + * + * Returns d->lpSA set to a SA or NULL. + */ +static LPSECURITY_ATTRIBUTES get_sa(struct my_sa_data *d) +{ + SID_IDENTIFIER_AUTHORITY sid_auth_world = SECURITY_WORLD_SID_AUTHORITY; +#define NR_EA (1) + EXPLICIT_ACCESS ea[NR_EA]; + DWORD dwResult; + + if (!AllocateAndInitializeSid(&sid_auth_world, 1, + SECURITY_WORLD_RID, 0,0,0,0,0,0,0, + &d->pEveryoneSID)) { + DWORD gle = GetLastError(); + trace2_data_intmax("ipc-debug", NULL, "alloc-world-sid/gle", + (intmax_t)gle); + goto fail; + } + + memset(ea, 0, NR_EA * sizeof(EXPLICIT_ACCESS)); + + ea[0].grfAccessPermissions = GENERIC_READ | GENERIC_WRITE; + ea[0].grfAccessMode = SET_ACCESS; + ea[0].grfInheritance = NO_INHERITANCE; + ea[0].Trustee.MultipleTrusteeOperation = NO_MULTIPLE_TRUSTEE; + ea[0].Trustee.TrusteeForm = TRUSTEE_IS_SID; + ea[0].Trustee.TrusteeType = TRUSTEE_IS_WELL_KNOWN_GROUP; + ea[0].Trustee.ptstrName = (LPTSTR)d->pEveryoneSID; + + dwResult = SetEntriesInAcl(NR_EA, ea, NULL, &d->pACL); + if (dwResult != ERROR_SUCCESS) { + DWORD gle = GetLastError(); + trace2_data_intmax("ipc-debug", NULL, "set-acl-entry/gle", + (intmax_t)gle); + trace2_data_intmax("ipc-debug", NULL, "set-acl-entry/dw", + (intmax_t)dwResult); + goto fail; + } + + d->pSD = (PSECURITY_DESCRIPTOR)LocalAlloc( + LPTR, SECURITY_DESCRIPTOR_MIN_LENGTH); + if (!InitializeSecurityDescriptor(d->pSD, SECURITY_DESCRIPTOR_REVISION)) { + DWORD gle = GetLastError(); + trace2_data_intmax("ipc-debug", NULL, "init-sd/gle", (intmax_t)gle); + goto fail; + } + + if (!SetSecurityDescriptorDacl(d->pSD, TRUE, d->pACL, FALSE)) { + DWORD gle = GetLastError(); + trace2_data_intmax("ipc-debug", NULL, "set-sd-dacl/gle", (intmax_t)gle); + goto fail; + } + + d->lpSA = (LPSECURITY_ATTRIBUTES)LocalAlloc(LPTR, sizeof(SECURITY_ATTRIBUTES)); + d->lpSA->nLength = sizeof(SECURITY_ATTRIBUTES); + d->lpSA->lpSecurityDescriptor = d->pSD; + d->lpSA->bInheritHandle = FALSE; + + return d->lpSA; + +fail: + release_sa(d); + return NULL; +} + static HANDLE create_new_pipe(wchar_t *wpath, int is_first) { HANDLE hPipe; DWORD dwOpenMode, dwPipeMode; - LPSECURITY_ATTRIBUTES lpsa = NULL; + struct my_sa_data my_sa_data; + + init_sa(&my_sa_data); dwOpenMode = PIPE_ACCESS_INBOUND | PIPE_ACCESS_OUTBOUND | FILE_FLAG_OVERLAPPED; @@ -612,20 +735,15 @@ static HANDLE create_new_pipe(wchar_t *wpath, int is_first) * set the ACL / Security Attributes on the named * pipe; subsequent instances inherit and cannot * change them. - * - * TODO Should we allow the application layer to - * specify security attributes, such as `LocalService` - * or `LocalSystem`, when we create the named pipe? - * This question is probably not important when the - * daemon is started by a foreground user process and - * only needs to talk to the current user, but may be - * if the daemon is run via the Control Panel as a - * System Service. */ + get_sa(&my_sa_data); } hPipe = CreateNamedPipeW(wpath, dwOpenMode, dwPipeMode, - PIPE_UNLIMITED_INSTANCES, 1024, 1024, 0, lpsa); + PIPE_UNLIMITED_INSTANCES, 1024, 1024, 0, + my_sa_data.lpSA); + + release_sa(&my_sa_data); return hPipe; } From patchwork Mon Sep 20 15:36:17 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Hostetler X-Patchwork-Id: 12505713 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 97CD3C433EF for ; Mon, 20 Sep 2021 15:36:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 80136610A3 for ; Mon, 20 Sep 2021 15:36:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237821AbhITPh7 (ORCPT ); Mon, 20 Sep 2021 11:37:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39374 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237285AbhITPhw (ORCPT ); Mon, 20 Sep 2021 11:37:52 -0400 Received: from mail-wr1-x430.google.com (mail-wr1-x430.google.com [IPv6:2a00:1450:4864:20::430]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 828B5C061762 for ; Mon, 20 Sep 2021 08:36:25 -0700 (PDT) Received: by mail-wr1-x430.google.com with SMTP id q11so30913500wrr.9 for ; Mon, 20 Sep 2021 08:36:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=IAw+cfXd7Fqp1Ld/tYY65l9gNk65pIHul9bcKmgli2Y=; b=bENUrUHbIbLXUCIiT708ay0qz/G+khF4HihJ03/i86CeOACPjZAgzu2avn4JW5M9FL O52CprXJHUti51c2cKL5h9JnCIAbey6T+DLOpEZBwe8QR4joEWW/LSZ01VI1OXKCN6vY 3MGCFZMAWbB8+K7AcVbOFgpoZ/U3Y+g3w9+AAopFoDNICquc+Y+OYFajozh3IcZQgZ43 LBGxCNjDqPevNWeIb2zZbFwajvpHZfJgjpRt+HjZdeFt7IWYRjoFiCyLdiAKFLJDlfAx TonKoCMwYck+AjZtQdyqDmFp5Pf03NKOVx6AdOR5mdy84/5RolkbNS4odGWN39/5hZeb kQbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=IAw+cfXd7Fqp1Ld/tYY65l9gNk65pIHul9bcKmgli2Y=; b=skljWRIGAP7YYHpj+yJ1LEHKVfWmyrPRLcyotfRrH3lmUpjHQtUdHlsnMH8PQnjvxF KGnNYTkNwdADg3X7uJq+Po3xXLgiopUq4wcAiC9u3c6xJLmrc3YTyjaXiXKIo2Oa9O5l ++xH4XY1iyXjX4kf23hsRmRFk+1Px9eevyqN6wkRA25ifTHukEVXXJ9M4iHzKKkJY2tZ MQiZrPzLu1KYB3B7B1C/OK+AKUVguKS98cY5hMfbVuVkkKmsbuhdEUrCe1fNroH/g4kn suLd5SCkDa1J3dftCdrXUz2+UC4YTC1oW71nEeLsXImfs+83G0uDasE2Iv7Npa/3oZ3a esow== X-Gm-Message-State: AOAM530LFF5dsJy5X5oA4v5YaAcG8GOe5iH3rjZSmcM4jbbhtnhIUIDD 2Jcb2OpCcD1diJRUCSBjnZL1IBc4EXo= X-Google-Smtp-Source: ABdhPJw85pYbK1hiKIchitTMOlmijpUXLIVI9MlIDWR5G50LR30SuzxVTygB6F+gXFeXVLv3Ock6SQ== X-Received: by 2002:a1c:7515:: with SMTP id o21mr25261149wmc.150.1632152184105; Mon, 20 Sep 2021 08:36:24 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id i18sm16284652wrn.64.2021.09.20.08.36.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Sep 2021 08:36:23 -0700 (PDT) Message-Id: <0822118c4b50ddd3668ded97a89cab2c47895078.1632152178.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Mon, 20 Sep 2021 15:36:17 +0000 Subject: [PATCH v2 6/7] run-command: create start_bg_command Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Eric Sunshine , Taylor Blau , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Jeff Hostetler , Carlo Arenas , Jeff Hostetler , Jeff Hostetler Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jeff Hostetler From: Jeff Hostetler Create a variation of `run_command()` and `start_command()` to launch a command into the background and optionally wait for it to become "ready" before returning. Signed-off-by: Jeff Hostetler --- run-command.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++ run-command.h | 57 ++++++++++++++++++++++ 2 files changed, 186 insertions(+) diff --git a/run-command.c b/run-command.c index 3e4e082e94d..76bbef9d96d 100644 --- a/run-command.c +++ b/run-command.c @@ -1901,3 +1901,132 @@ void prepare_other_repo_env(struct strvec *env_array, const char *new_git_dir) } strvec_pushf(env_array, "%s=%s", GIT_DIR_ENVIRONMENT, new_git_dir); } + +enum start_bg_result start_bg_command(struct child_process *cmd, + start_bg_wait_cb *wait_cb, + void *cb_data, + unsigned int timeout_sec) +{ + enum start_bg_result sbgr = SBGR_ERROR; + int ret; + int wait_status; + pid_t pid_seen; + time_t time_limit; + + /* + * We do not allow clean-on-exit because the child process + * should persist in the background and possibly/probably + * after this process exits. So we don't want to kill the + * child during our atexit routine. + */ + if (cmd->clean_on_exit) + BUG("start_bg_command() does not allow non-zero clean_on_exit"); + + if (!cmd->trace2_child_class) + cmd->trace2_child_class = "background"; + + ret = start_command(cmd); + if (ret) { + /* + * We assume that if `start_command()` fails, we + * either get a complete `trace2_child_start() / + * trace2_child_exit()` pair or it fails before the + * `trace2_child_start()` is emitted, so we do not + * need to worry about it here. + * + * We also assume that `start_command()` does not add + * us to the cleanup list. And that it calls + * calls `child_process_clear()`. + */ + sbgr = SBGR_ERROR; + goto done; + } + + time(&time_limit); + time_limit += timeout_sec; + +wait: + pid_seen = waitpid(cmd->pid, &wait_status, WNOHANG); + + if (!pid_seen) { + /* + * The child is currently running. Ask the callback + * if the child is ready to do work or whether we + * should keep waiting for it to boot up. + */ + ret = (*wait_cb)(cmd, cb_data); + if (!ret) { + /* + * The child is running and "ready". + */ + trace2_child_ready(cmd, "ready"); + sbgr = SBGR_READY; + goto done; + } else if (ret > 0) { + /* + * The callback said to give it more time to boot up + * (subject to our timeout limit). + */ + time_t now; + + time(&now); + if (now < time_limit) + goto wait; + + /* + * Our timeout has expired. We don't try to + * kill the child, but rather let it continue + * (hopefully) trying to startup. + */ + trace2_child_ready(cmd, "timeout"); + sbgr = SBGR_TIMEOUT; + goto done; + } else { + /* + * The cb gave up on this child. It is still running, + * but our cb got an error trying to probe it. + */ + trace2_child_ready(cmd, "error"); + sbgr = SBGR_CB_ERROR; + goto done; + } + } + + else if (pid_seen == cmd->pid) { + int child_code = -1; + + /* + * The child started, but exited or was terminated + * before becoming "ready". + * + * We try to match the behavior of `wait_or_whine()` + * WRT the handling of WIFSIGNALED() and WIFEXITED() + * and convert the child's status to a return code for + * tracing purposes and emit the `trace2_child_exit()` + * event. + * + * We do not want the wait_or_whine() error message + * because we will be called by client-side library + * routines. + */ + if (WIFEXITED(wait_status)) + child_code = WEXITSTATUS(wait_status); + else if (WIFSIGNALED(wait_status)) + child_code = WTERMSIG(wait_status) + 128; + trace2_child_exit(cmd, child_code); + + sbgr = SBGR_DIED; + goto done; + } + + else if (pid_seen < 0 && errno == EINTR) + goto wait; + + trace2_child_exit(cmd, -1); + sbgr = SBGR_ERROR; + +done: + child_process_clear(cmd); + invalidate_lstat_cache(); + return sbgr; +} diff --git a/run-command.h b/run-command.h index af1296769f9..17b1b80c3d7 100644 --- a/run-command.h +++ b/run-command.h @@ -496,4 +496,61 @@ int run_processes_parallel_tr2(int n, get_next_task_fn, start_failure_fn, */ void prepare_other_repo_env(struct strvec *env_array, const char *new_git_dir); +/** + * Possible return values for start_bg_command(). + */ +enum start_bg_result { + /* child process is "ready" */ + SBGR_READY = 0, + + /* child process could not be started */ + SBGR_ERROR, + + /* callback error when testing for "ready" */ + SBGR_CB_ERROR, + + /* timeout expired waiting for child to become "ready" */ + SBGR_TIMEOUT, + + /* child process exited or was signalled before becomming "ready" */ + SBGR_DIED, +}; + +/** + * Callback used by start_bg_command() to ask whether the + * child process is ready or needs more time to become "ready". + * + * The callback will receive the cmd and cb_data arguments given to + * start_bg_command(). + * + * Returns 1 is child needs more time (subject to the requested timeout). + * Returns 0 if child is "ready". + * Returns -1 on any error and cause start_bg_command() to also error out. + */ +typedef int(start_bg_wait_cb)(const struct child_process *cmd, void *cb_data); + +/** + * Start a command in the background. Wait long enough for the child + * to become "ready" (as defined by the provided callback). Capture + * immediate errors (like failure to start) and any immediate exit + * status (such as a shutdown/signal before the child became "ready") + * and return this like start_command(). + * + * We run a custom wait loop using the provided callback to wait for + * the child to start and become "ready". This is limited by the given + * timeout value. + * + * If the child does successfully start and become "ready", we orphan + * it into the background. + * + * The caller must not call finish_command(). + * + * The opaque cb_data argument will be forwarded to the callback for + * any instance data that it might require. This may be NULL. + */ +enum start_bg_result start_bg_command(struct child_process *cmd, + start_bg_wait_cb *wait_cb, + void *cb_data, + unsigned int timeout_sec); + #endif From patchwork Mon Sep 20 15:36:18 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Hostetler X-Patchwork-Id: 12505717 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1E0E2C433EF for ; Mon, 20 Sep 2021 15:36:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0394E61107 for ; Mon, 20 Sep 2021 15:36:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238244AbhITPiC (ORCPT ); Mon, 20 Sep 2021 11:38:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39382 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237296AbhITPhx (ORCPT ); Mon, 20 Sep 2021 11:37:53 -0400 Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 43BE9C061764 for ; Mon, 20 Sep 2021 08:36:26 -0700 (PDT) Received: by mail-wr1-x435.google.com with SMTP id x6so30881763wrv.13 for ; Mon, 20 Sep 2021 08:36:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=0leNCrRjt7aDENNOAgS9SSnLE/ZBsnwDX69mMwa0hZE=; b=Vi56cQco+jMiEYxLRdRH/OVL5DTsxFWq4GsttCq+D1h/X/CodJgwWlD76vkbcVBqYG 9Ct1dQUjQOVfvKTWVdUigESYQ7Ruh7edGH0yAVyFZRply72DP0grk1M1eVabYeXKWqi9 JuigCxtHtwMhrTrlgh06LK1KhXb5zQ4TkaHpMhRWLAWd/wCS/vE84Y8W98yfp7S4KVvE tbtkI4mkBQYbr2k5bB8tFPLjtrQZ4dL/VdHFxnfUsXsHYT4RQEjn3mulaLqnPpGgYsRZ Vsitip5u9UNtvpiR3R1UYo87e/AXMHWA170EK1eMnJn9N76KjBah3ND6b1WGaUu3Y3cb h7ag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=0leNCrRjt7aDENNOAgS9SSnLE/ZBsnwDX69mMwa0hZE=; b=5MPC/vroowsPTu7SepilzIHo8Jyr6vUION36e/qroLIGfXSXkTB4mSU9u2fopDcMCO 8Y+R9uM9Xcua94px+dkPPsERttTOl2/WuMnKfeEkCfIQXTnCbiZq8ZvjYBHkOHhyxFjJ G1Nz+VO0EggyLBCyUXjD2JsWyT49ZAA8Q4OJQ4ScMRAYwXW7s6rk9XqL9IhJVDEa68DQ XU/XJlasXddHhYou9m2A6yEWFco0RoLWbF4pqyY8LbzL8Go/vce6YOJ/h39nzfUF5p0U U3XbTLutWddez7RAnvbz/DqeT4FtGMAc3TXfgIYivvYfVL6sIDNjLomA4ln5uAkoWnjT j4dQ== X-Gm-Message-State: AOAM532GYKIspIO/8nhgCsnSqFpz+mUn0k8FpfI9j+AP1F8FPMNGsdv4 R9BHiU7UYS8+6AHYPz5QoTeIzmL5NJg= X-Google-Smtp-Source: ABdhPJzMcQBKLJ27mNlNyupi4JUy2Op8jgYP6JX2L6TI+Tp0/BWoJQZDMQ7gBnR57c1ALYi4oFfD0A== X-Received: by 2002:a05:600c:3797:: with SMTP id o23mr15888852wmr.111.1632152184820; Mon, 20 Sep 2021 08:36:24 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id w5sm16416098wra.87.2021.09.20.08.36.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Sep 2021 08:36:24 -0700 (PDT) Message-Id: <6b7a058284b93fab52458b12a6aede5e8aed6652.1632152179.git.gitgitgadget@gmail.com> In-Reply-To: References: 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 MIME-Version: 1.0 To: git@vger.kernel.org Cc: Eric Sunshine , Taylor Blau , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Jeff Hostetler , Carlo Arenas , Jeff Hostetler , Jeff Hostetler Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Jeff Hostetler From: Jeff Hostetler Convert test helper to use `start_bg_command()` when spawning a server daemon in the background rather than blocks of platform-specific code. Also, while here, remove _() translation around error messages since this is a test helper and not Git code. Signed-off-by: Jeff Hostetler --- t/helper/test-simple-ipc.c | 199 ++++++++----------------------------- 1 file changed, 43 insertions(+), 156 deletions(-) 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"); } }