|From:||Andres Freund <andres(at)anarazel(dot)de>|
|To:||Chris Travers <chris(dot)travers(at)adjust(dot)com>|
|Subject:||Re: Proposal for Signal Detection Refactoring|
|Views:||Raw Message | Whole Thread | Download mbox|
On 2018-09-24 11:45:10 +0200, Chris Travers wrote:
> I did some more reading.
> On Mon, Sep 24, 2018 at 10:15 AM Chris Travers <chris(dot)travers(at)adjust(dot)com>
> > First, thanks for taking the time to write this. Its very helpful.
> > Additional thoughts inline.
> > On Mon, Sep 24, 2018 at 2:12 AM Michael Paquier <michael(at)paquier(dot)xyz>
> > wrote:
> >> There could be value in refactoring things so as all the *Pending flags
> >> of miscadmin.h get stored into one single volatile sig_atomic_t which
> >> uses bit-wise markers, as that's at least 4 bytes because that's stored
> >> as an int for most platforms and can be performed as an atomic operation
> >> safely across signals (If my memory is right;) ). And this leaves a lot
> >> of room for future flags.
> > Yeah I will look into this.
> Ok so having looked into this a bit more....
> It looks like to be strictly conforming, you can't just use a series of
> flags because neither C 89 or 99 guarantee that sig_atomic_t is read/write
> round-trip safe in signal handlers. In other words, if you read, are
> pre-empted by another signal, and then write, you may clobber what the
> other signal handler did to the variable. So you need atomics, which are
> What I would suggest instead at least for an initial approach is:
> 1. A struct of volatile bools statically stored
> 2. macros for accessing/setting/clearing flags
> 3. Consistent use of these macros throughout the codebase.
> To make your solution work it looks like we'd need C11 atomics which would
> be nice and maybe at some point we decide to allow newer feature, or we
> could wrap this itself in checks for C11 features and provide atomic flags
> in the future. It seems that the above solution would strictly comply to
> C89 and pose no concurrency issues.
It's certainly *NOT* ok to use atomics in signal handlers
indiscriminately, on some hardware / configure option combinations
they're backed by spinlocks (or even semaphores) and thus *NOT*
This doesn't seem to solve an actual problem, why are we discussing
changing this? What'd be measurably improved, worth the cost of making
backpatching more painful?
|Next Message||Tom Lane||2018-09-24 17:18:47||Re: Allowing printf("%m") only where it actually works|
|Previous Message||Elvis Pranskevichus||2018-09-24 16:28:49||Re: [HACKERS] [PATCH v2] Add and report the new "session_read_only" GUC pseudo-variable.|