pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Nathan Bossart <nathandbossart@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Andy Fan <zhihuifan1213@163.com>
Cc: pgsql-hackers@postgresql.org, noah@leadboat.com, Robert Haas <robertmhaas@gmail.com>
Subject: Re: common signal handler protection
Date: Wed, 7 Feb 2024 14:12:08 -0600
Message-ID: <20240207201208.GA279677@nathanxps13> (raw)
In-Reply-To: <20240207184050.rkvpuudq7huijmkb@awork3.anarazel.de>
References: <20231121212008.GA3742740@nathanxps13>
	<20240207023941.GA67817@nathanxps13>
	<20240207024853.pg55gxzmqxi3nrmr@awork3.anarazel.de>
	<20240207170650.GA80671@nathanxps13>
	<20240207171554.GA153000@nathanxps13>
	<20240207184050.rkvpuudq7huijmkb@awork3.anarazel.de>

On Wed, Feb 07, 2024 at 10:40:50AM -0800, Andres Freund wrote:
> On 2024-02-07 11:15:54 -0600, Nathan Bossart wrote:
>> Perhaps we should add a file global bool that is only set during
>> wrapper_handler().  Then we could Assert() or elog(ERROR, ...) if
>> pqsignal() is called with it set.
> 
> In older branches that might have been harder (due to forking from a signal
> handler and non-fatal errors thrown from signal handlers), but these days I
> think that should work.
> 
> FWIW, I don't think elog(ERROR) would be appropriate, that'd be jumping out of
> a signal handler :)

*facepalm*  Yes.

> If it were just for the purpose of avoiding the issue you brought up it might
> not quite be worth it - but there are a lot of things we want to forbid in a
> signal handler. Memory allocations, acquiring locks, throwing non-panic
> errors, etc. That's one of the main reasons I like a common wrapper signal
> handler.
> 
> Which reminded me of https://postgr.es/m/87msstvper.fsf%40163.com - the set of
> things we want to forbid are similar. I'm not sure there's really room to
> harmonize things, but I thought I'd raise it.
> 
> Perhaps we should make the state a bitmap and have a single
>   AssertNotInState(HOLDING_SPINLOCK | IN_SIGNALHANDLER)

Seems worth a try.  I'll go ahead and proceed with these patches and leave
this improvement for another thread.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com





view thread (18+ messages)  latest in thread

Message-ID: <20240207201208.GA279677@nathanxps13>
Permalink:  ../20240207201208.GA279677@nathanxps13/
Also on:    postgresql.org/message-id/20240207201208.GA279677@nathanxps13

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: nathandbossart@gmail.com, andres@anarazel.de, zhihuifan1213@163.com, robertmhaas@gmail.com
  Subject: Re: common signal handler protection
  In-Reply-To: <20240207201208.GA279677@nathanxps13>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox