Re: [PATCH] trace2: intercept all common signals

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 10, 2024 at 10:22 AM Emily Shaffer <emilyshaffer@xxxxxxxxxx> wrote:
>
> From: Emily Shaffer <nasamuffin@xxxxxxxxxx>
>
> We already use trace2 to find out about unexpected pipe breakages, which
> is nice for detecting bugs or system problems, by adding a handler for
> SIGPIPE which simply writes a trace2 line. However, there are a handful
> of other common signals which we might want to snoop on:
>
>  - SIGINT, SIGTERM, or SIGQUIT, when a user manually cancels a command in
>    frustration or mistake (via Ctrl-C, Ctrl-D, or `kill`)
>  - SIGHUP, when the network closes unexpectedly (indicating there may be
>    a problem to solve)
>
> There are lots more signals which we might find useful later, but at
> least let's teach trace2 to report these egregious ones. Conveniently,
> they're also already covered by the `_common` variants in sigchain.[ch].
>
> Sigchain itself is already tested via helper/test-sigchain.c, and trace2
> is tested in a number of places - let's also add tests demonstrating
> that sigchain + trace2 works correctly.
>
> Signed-off-by: Emily Shaffer <nasamuffin@xxxxxxxxxx>
> ---

Missed including the CI results. They're passing[1] with the exception
of the osx-gcc run, which seems to also be failing on the latest
'master'[2] and looks to be failing in setup rather than in test run.

1: https://github.com/nasamuffin/git/actions/runs/9035666915
2: https://github.com/nasamuffin/git/actions/runs/9036080205/job/24832209129

>  t/helper/test-trace2.c   | 17 +++++++++++++++++
>  t/t0210-trace2-normal.sh | 22 ++++++++++++++++++++++
>  trace2.c                 |  2 +-
>  3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c
> index 1adac29a57..8970956ea8 100644
> --- a/t/helper/test-trace2.c
> +++ b/t/helper/test-trace2.c
> @@ -231,6 +231,22 @@ static int ut_010bug_BUG(int argc UNUSED, const char **argv UNUSED)
>         BUG("a %s message", "BUG");
>  }
>
> +static int ut_011signal(int argc, const char **argv)
> +{
> +       const char *usage_error = "expect <bool common>";
> +       int common = 0;
> +
> +       if (argc != 1 || get_i(&common, argv[0]))
> +               die("%s", usage_error);
> +
> +       /*
> +        * There is no strong reason SIGSEGV is ignored by trace2 - it's just
> +        * not included by sigchain_push_common().
> +        */
> +       raise(common ? SIGINT : SIGSEGV);
> +       return 0; /*unreachable*/
> +}
> +
>  /*
>   * Single-threaded timer test.  Create several intervals using the
>   * TEST1 timer.  The test script can verify that an aggregate Trace2
> @@ -482,6 +498,7 @@ static struct unit_test ut_table[] = {
>         { ut_008bug,      "008bug",    "" },
>         { ut_009bug_BUG,  "009bug_BUG","" },
>         { ut_010bug_BUG,  "010bug_BUG","" },
> +       { ut_011signal,   "011signal","" },
>
>         { ut_100timer,    "100timer",  "<count> <ms_delay>" },
>         { ut_101timer,    "101timer",  "<count> <ms_delay> <threads>" },
> diff --git a/t/t0210-trace2-normal.sh b/t/t0210-trace2-normal.sh
> index c312657a12..c34ccc518c 100755
> --- a/t/t0210-trace2-normal.sh
> +++ b/t/t0210-trace2-normal.sh
> @@ -244,6 +244,28 @@ test_expect_success 'bug messages followed by BUG() are written to trace2' '
>         test_cmp expect actual
>  '
>
> +test_expect_success 'trace2 reports common signals' '
> +       test_when_finished "rm trace.normal actual" &&
> +
> +       # signals are fatal, so expect this to fail
> +       ! env GIT_TRACE2="$(pwd)/trace.normal" test-tool trace2 011signal 1 &&
> +
> +       perl "$TEST_DIRECTORY/t0210/scrub_normal.perl" <trace.normal >actual &&
> +
> +       test_grep "signal elapsed:" actual
> +'
> +
> +test_expect_success 'trace2 ignores uncommon signals' '
> +       test_when_finished "rm trace.normal actual" &&
> +
> +       # signals are fatal, so expect this to fail
> +       ! env GIT_TRACE2="$(pwd)/trace.normal" test-tool trace2 011signal 0 &&
> +
> +       perl "$TEST_DIRECTORY/t0210/scrub_normal.perl" <trace.normal >actual &&
> +
> +       ! test_grep "signal elapsed:" actual
> +'
> +
>  sane_unset GIT_TRACE2_BRIEF
>
>  # Now test without environment variables and get all Trace2 settings
> diff --git a/trace2.c b/trace2.c
> index f894532d05..3692010f5d 100644
> --- a/trace2.c
> +++ b/trace2.c
> @@ -231,7 +231,7 @@ void trace2_initialize_fl(const char *file, int line)
>         tr2_sid_get();
>
>         atexit(tr2main_atexit_handler);
> -       sigchain_push(SIGPIPE, tr2main_signal_handler);
> +       sigchain_push_common(tr2main_signal_handler);
>         tr2tls_init();
>
>         /*
> --
> 2.45.0.118.g7fe29c98d7-goog
>





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux