Re: [PATCH] trace2: intercept all common signals

> From: Emily Shaffer <nasamuffin@google.com>
>
> 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>
> ---
>  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 &&

> 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();
>  
>  	/*