Re: [PATCH v4 8/8] progress.c: add & assert a "global_progress" variable

On Mon, Oct 25 2021, Johannes Sixt wrote:

Am 25.10.21 um 13:25 schrieb Ævar Arnfjörð Bjarmason:

quoted

Once we have a
global current progress object we'll be able to update that object via
SIGALRM, this will cover cases where we're busy, but either haven't
invoked our first display_progress() yet, or the time between
display_progress() is too long. See [3] for early code to do that.

This can't be a goal. You cannot do serious stuff in a signal handler.
Don't proliferate complicated signal handlers.

I'd missed some of the nuances with signal safety when I hacked that up
initially. For the purposes of this patch the commit message is
discussing a POC that suggests an interesting direction to go in.

My current implementation of this which I've run with for some time
relies on undefined behavior and glibc being forgiving, but for any
future submission I'll make sure it's signal-safe.

In particular to format a progress line etc. we we need sprintf() and
the like, but to have a "stalled" message we can pre-format that at
start_progress() time, and inside the signal handler all we'll need is a
write(), which should be safe in most places...

On top of that, our SIGALRM handler on Windows runs in its own thread
and can't do more than set a volatile sigatomic_t (and even that is
undefined behavior, strictly speaking).

... but we may need to disable it entirely on some platforms, I didn't
know about that Windows edge case.

Anyway, for this patch having the BUG() is useful even if we never went
for that.