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 >
