From: Tom Lane <tgl@sss.pgh.pa.us>
To: Stephen Frost <sfrost@snowman.net>
cc: pgsql-hackers@lists.postgresql.org
Subject: Re: SIGQUIT handling, redux
In-reply-to: <20200909143900.GQ29590@tamriel.snowman.net>
References: <1850884.1599601164@sss.pgh.pa.us>
 <20200909143900.GQ29590@tamriel.snowman.net>
Comments: In-reply-to Stephen Frost <sfrost@snowman.net>
	message dated "Wed, 09 Sep 2020 10:39:00 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <12862.1599662788.1@sss.pgh.pa.us>
Date: Wed, 09 Sep 2020 10:46:28 -0400
Message-ID: <12863.1599662788@sss.pgh.pa.us>
Precedence: bulk

Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> Of course, this is only safe if the SIGQUIT handler is safe to be invoked
>> anywhere, so I did a quick survey of backend signal handlers to see if
>> that is true.  I immediately found pgarch_exit() which surely is not.  It
>> turns out that Horiguchi-san already noticed that and proposed to fix it,
>> within the seemingly entirely unrelated patch series for a shared-memory
>> based stats collector (see patch 0001 in [2]).  I think we should go ahead
>> and commit that, and maybe even back-patch it.  There seems no good reason
>> for the archiver to treat SIGQUIT as nonfatal when other postmaster
>> children do not.

> As I mentioned over there, I agree that we should do this and we should
> further have the statistics collector also do so, which currently sets
> up SIGQUIT with ShutdownRequestPending() and in its loop decides it's
> fine to write out the stats file (which we're going to remove during
> recovery anyway...) and then call exit(0).

I noticed that that was different from everything else, but it's not
actually signal-unsafe, so it seems like a different topic from what
I'm on about at the moment.  I don't mind if you or somebody else
wants to change it, but I don't see it as a back-patchable bug fix.

> I also think we should
> back-patch these changes, as the commit mentioned in Horiguchi-san's
> patch for pgarch_exit() was.

Agreed; I'll go make it happen.

			regards, tom lane