Skip to content

Commit

Permalink
trace2: intercept all common signals
Browse files Browse the repository at this point in the history
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@google.com>
  • Loading branch information
Emily Shaffer authored and nasamuffin committed May 10, 2024
1 parent 0f3415f commit f4e6077
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 1 deletion.
17 changes: 17 additions & 0 deletions t/helper/test-trace2.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>" },
Expand Down
22 changes: 22 additions & 0 deletions t/t0210-trace2-normal.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion trace2.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();

/*
Expand Down

0 comments on commit f4e6077

Please sign in to comment.