Re: [HACKERS] possible self-deadlock window after bad ProcessStartupPacket

On 20/07/18 18:03, Andres Freund wrote:
> On 2018-07-20 11:53:32 +0300, Heikki Linnakangas wrote:
>> But I think we have consensus on replacing the exit(2) calls
>> with _exit(2). If we do just that, it would be better than the status quo,
>> even if it doesn't completely fix the problem. This would prevent the case
>> that Asim reported, for starters.
>
> Right. You're planning to backpatch?

Yeah. Committed and back-patched.

>> With _exit(), I think we wouldn't really need the "PG_SETMASK(&BlockSig);"
>> calls in the aux process *_quickdie handlers that don't do anything else
>> than call _exit(). But I didn't dare to remove them yet.
>
> I think we should remove it at least in master.

Removed.

> It's much less the exit() that's unsafe, than the callbacks themselves,
> right? Perhaps just restate that we wouldn't want to trigger atexit
> processing due to signal safety?

Well, in principle exit() is unsafe too, although you're right that in
practice it's more likely the callbacks that would cause trouble. I
reworded the comment to put more emphasis on the callbacks.

On 23/07/18 17:34, Robert Haas wrote:
> +1 for trying to improve this by using _exit rather than exit, and for
> not letting the perfect be the enemy of the good.
>
> But -1 for copying the language "if some idiot DBA sends a manual
> SIGQUIT to a random backend". I think that phrase could be deleted
> from this comment -- and all of the other places where this comment
> appears already today -- without losing any useful informational
> content. Or it could be rephrased to "if this process receives a
> SIGQUIT". It's just not necessary to call somebody an idiot to
> communicate the point.

Rephrased to be less offensive.

So, with this commit, the various SIGQUIT quickdie handlers are in a
better shape. The regular backend's quickdie() handler still calls
ereport(), which is not safe, but it's a step in the right direction.
And we haven't addressed the original complaint, which was actually
about startup_die(), not quickdie().

What should we do about startup_die()? Ideas?

- Heikki