Re: Cleaning up historical portability baggage

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Cleaning up historical portability baggage
Date: 2022-08-16 04:14:27
Message-ID: CA+hUKGL5CnwUEt655dymhcMf35vO4HY_MAuABr1=Nqat_RjRLA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 16, 2022 at 1:16 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > But let's suppose we want to play by a timid interpretation of that page's
> > "do not issue low-level or STDIO.H I/O routines". It also says that SIGINT
> > is special and runs the handler in a new thread (in a big warning box
> > because that has other hazards that would break other kinds of code). Well,
> > we *know* it's safe to unlink files in another thread... so... how cheesy
> > would it be if we just did raise(SIGINT) in the real handlers?
>
> Not quite sure I understand. You're proposing to raise(SIGINT) for all other
> handlers, so that signal_remove_temp() gets called in another thread, because
> we assume that'd be safe because doing file IO in other threads is safe? That
> assumes the signal handler invocation infrastructure isn't the problem...

That's what I was thinking about, yeah. But after some more reading,
now I'm wondering if we'd even need to do that, or what I'm missing.
The 6 listed signals in the manual are SIGABRT, SIGFPE, SIGILL,
SIGINT, SIGSEGV and SIGTERM (the 6 required by C). We want to run
signal_remove_temp() on SIGHUP (doesn't exist, we made it up), SIGINT,
SIGPIPE (doesn't exist, we made it up), and SIGTERM (exists for C spec
compliance but will never be raised by the system according to the
manual, and we don't raise it ourselves IIUC). So the only case we
actually have to consider is SIGINT, and SIGINT handlers run in a
thread, so if we assume it is therefore exempt from those
very-hard-to-comply-with rules, aren't we good already? What am I
missing?

> Looks like we could register a "native" ctrl-c handler:
> https://docs.microsoft.com/en-us/windows/console/setconsolectrlhandler
> they're documented to run in a different thread, but without any of the
> file-io warnings.
> https://docs.microsoft.com/en-us/windows/console/handlerroutine

Sounds better in general, considering the extreme constraints of the
signal system, but it'd be nice to see if the current system is truly
unsafe before writing more alien code.

Someone who wants to handle more than one SIGINT would certainly need
to consider that, because there doesn't seem to be a race-free way to
reinstall the signal handler when you receive it[1]. Races aside, for
any signal except SIGINT (assuming the above-mentioned exemption),
you're probably also not even allowed to try because raise() might be
a system call and they're banned. Fortunately we don't care, we
wanted SIG_DFL next anyway.

[1] https://wiki.sei.cmu.edu/confluence/display/c/SIG01-C.+Understand+implementation-specific+details+regarding+signal+handler+persistence

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2022-08-16 05:02:23 Re: Support logical replication of global object commands
Previous Message Andrey Borodin 2022-08-16 03:57:54 Logical WAL sender unresponsive during decoding commit