Date: Wed, 15 Jan 2025 14:56:44 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Fujii Masao <masao.fujii@oss.nttdata.com>,
	Andy Fan <zhihuifan1213@163.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: convert libpgport's pqsignal() to a void function
Message-ID: <Z4ghDB-Bazbz7W8o@nathan>
References: <Z4chOKfnthRH71mw@nathan>
 <1164438.1736910166@sss.pgh.pa.us>
 <Z4cyRX-hqfXREYKC@nathan>
 <1171432.1736914085@sss.pgh.pa.us>
 <Z4gJTtG4ZX9vXl-y@nathan>
 <CA+hUKGL0NyB03uKrNqN0z1M=cwju_b_RJLgxtVPU4f0h8-8sZg@mail.gmail.com>
 <Z4gQxiLQrf5OLf18@nathan>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Z4gQxiLQrf5OLf18@nathan>
Archived-At: <https://www.postgresql.org/message-id/Z4ghDB-Bazbz7W8o%40nathan>
Precedence: bulk

On Wed, Jan 15, 2025 at 01:47:18PM -0600, Nathan Bossart wrote:
> On Thu, Jan 16, 2025 at 08:21:08AM +1300, Thomas Munro wrote:
>> Could be due to calling native signal() with a signal number other
>> than the 6 values required to work by the C standard?
> 
> Looking closer, that probably makes more sense than my SIG_ERR redefinition
> theory.  If that assertion is getting hit, that means signal() _is_
> returning SIG_ERR (either the system one or our redefined version), and it
> looks like it's pretty common to use -1 for SIG_ERR.  That'd only affect
> Windows frontend programs, but it still sounds scary.  I'll try getting
> more details about the error with some custom cfbot runs.

I think this is what's happening.  cfbot's test failures are caused by
initdb's setup_signals().  The call to pqsignal(SIGHUP, trapsig) seems to
fail because SIGHUP isn't a real signal number, just something that's made
up in win32_port.h.  This SIGHUP definition was added by commit 12c9423 in
May 2003, then the pqsignal(SIGHUP, ...) call was added in initdb by commit
279598b in November 2003, but it might not have been broken at that time
because it doesn't look like initdb.c included the SIGHUP #define.  In any
case, I think this has been broken since at least commit ed9b360 (November
2017), if not earlier.

Perhaps we should surround all those extra signal #defines in win32_port.h
with an #ifndef FRONTEND.  initdb seems to be good about avoiding
pqsignal() calls if the signal doesn't exist, but I wouldn't be surprised
if there are other frontend programs that are not so cautious.  I'll give
it a try on cfbot and see what breaks...

-- 
nathan