Re: [PATCH] pager: exit without error on SIGPIPE

Ævar Arnfjörð Bjarmason [off-list ref] writes:

Getting back to the point, whatever anyone thinks of returning SIGHUP as
we do now or not, I think it's bonkers to ignore SIGHUP and *then*
return a non-zero *only in the non-atexit case*.

That just means that if you do have a broken pager you're going to get
flaky exits depending on the state of our flushed buffers, who's going
to be helped by such a fickle exit code?

So if we're going to change the behavior to not return SIGHUP, and don't
want to refactor our entire atexit() handling in #2 to be guaranteed to
pass down the pager's exit code, I don't see how anything except the
approach of just exit(0) in that case makes sense, which is what Denton
Liu's patch initially suggested doing.

Then we are on the same page (assuming that all your HUPs are
PIPEs), as the "perhaps we could even exit with pager's error" was
my mistaken reaction to your "we have been losing pager's exit
status" message.

I do want to ignore, as I said in the message you are responding to,
the exit status of the pager, just like we ignore exit status of
some of the hooks that we spawn primarily for their side effects (as
opposed to the pre-* hooks whose status we do use to base our
decision on).

I guess we just want to take just a half of your [WIP/PATCH v2 5/5],
ignoring the return values from finish_command*() and exiting with 0
when we got SIGPIPE (that would mean that there will be no change on
the atexit codepath).  Unlike Denton's change directly on the current
codebase, the resulting code would clearly show that we only care about
the signal codepath, thanks to the refactoring [PATCH v2 1/5] has
done.

On Wed, Feb 03 2021, Junio C Hamano wrote:

Ævar Arnfjörð Bjarmason [off-list ref] writes:

quoted

Getting back to the point, whatever anyone thinks of returning SIGHUP as
we do now or not, I think it's bonkers to ignore SIGHUP and *then*
return a non-zero *only in the non-atexit case*.

That just means that if you do have a broken pager you're going to get
flaky exits depending on the state of our flushed buffers, who's going
to be helped by such a fickle exit code?

So if we're going to change the behavior to not return SIGHUP, and don't
want to refactor our entire atexit() handling in #2 to be guaranteed to
pass down the pager's exit code, I don't see how anything except the
approach of just exit(0) in that case makes sense, which is what Denton
Liu's patch initially suggested doing.

Then we are on the same page (assuming that all your HUPs are
PIPEs)

Yes, sorry, PBCAK :)

Am 03.02.21 um 03:54 schrieb Junio C Hamano:

I guess we just want to take just a half of your [WIP/PATCH v2 5/5],
ignoring the return values from finish_command*() and exiting with 0
when we got SIGPIPE (that would mean that there will be no change on
the atexit codepath).  Unlike Denton's change directly on the current
codebase, the resulting code would clearly show that we only care about
the signal codepath, thanks to the refactoring [PATCH v2 1/5] has
done.

Note though, that we cannot call exit() from a signal handler: it is not
async-signal safe.

https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03

-- Hannes