Re: [PATCH] trace2: intercept all common signals

From: Junio C Hamano <gitster@pobox.com>
Date: May 16, 2024, 16:32 UTC
Message-ID: <xmqqwmntra3f.fsf@gitster.g>
In-Reply-To: <20240516071127.GA83658@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 9 quoted lines

>   - the opposite approach might be: stop using any allocating functions
>     in the trace2 code. There's a certain simplicity there, even for
>     non-signal functions, that we know we're just touching a few
>     fixed-size buffers, and you can never create a weird DoS by tweaking
>     the tracing code. But it would mean rewriting a lot of it (including
>     json formatting stuff) without many of our usual strbuf niceties.
>
>     This is more or less the approach we take with error(), die(), etc,
>     which are built on vreportf() and its fixed buffer.

Would another approach be to add various trace2 functions that use strbuf() allocation a way to tell if they are called from a signal handing codepath, and punt (by doing nothing if needed, but hopefully we have enough slop in the buffer to say "hey we got interrupted so no more detailed report for you, sorry") if that is the case?

> So overall it is a pretty thorny problem, and for the most part we've
> just tried to keep what we do inside signal handlers to a minimum
> (usually cleanup, but even there we have to be careful not to do things
> like build up allocated paths for recursive removal).

Yes, I agree that it is the right approach to do very little in a signal handler.