SIGTTIN / SIGTTOU handling (was Re: BUG #15449)

Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> writes:
> On Wed, Nov 7, 2018 at 11:03 PM Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> (2018/11/06 19:50), Thomas Munro wrote:
>>> Why do bgwriter.c,
>>> startup.c, ... set SIGTTIN and SIGTTOU back to SIG_DFL, but not
>>> regular backends?

>> So, we should revert SIGUSR2 as well to default processing?

> I don't think it matters in practice, but it might be nice to restore
> that just for consistency. I'm not sure what to think about the TTIN,
> TTOU stuff; I don't understand job control well right now but I don't
> think it really applies to programs run by a PostgreSQL backend, so if
> we restore those it'd probably again be only for consistency. Then
> again, there may be a reason someone decided to ignore those in the
> postmaster + regular backends but not the various auxiliary processes.
> Anyone?

I dug around in our git history, and found that the inconsistency seems
to be largely my fault :-( although there's basically no discussion
anywhere about the decisions. According to git:

* The postmaster has SIG_IGN'd SIGTTIN and SIGTTOU as far back as our
history goes.

* In commit 212c905e2 (May 1998), Bruce added a reset to SIG_DFL when
launching a backend. No indication of why.

* In commit 140ddb78f (Jun 2001), Jan added pgstat.c with an explicit
SIG_IGN setting in the child process launch code. (That was redundant
given the postmaster code.)

* In commit dad8e410d (Aug 2001), I changed pgstat.c to use SIG_DFL,
probably to match the backend behavior.

* In commit 8e2998d8a (Feb 2002), I removed the SIG_DFL setting in
tcop.c, noting only that it was "unnecessary". Evidently I missed
the one in pgstat.c.

I think all the other child process launch code has just cargo-culted
those lines in from pgstat.c. The relevant lines have been touched
or moved quite a few times, so I might have misidentified some things,
but it looks like those commits were the ones that changed behavior.

I think that really the sane behavior ought to be for the postmaster
to SIG_IGN these signals as it always has, and for nothing else to
touch them at all. This would mean that a standalone backend would
have default handling for these signals, which seems reasonable, and
that they'd be ignored by all processes in a normal postmaster
environment, not just some of them; which also seems reasonable.

It certainly is weird to have a mix of SIG_IGN and SIG_DFL in a
postmaster process collection as we do now, and the processes most
likely to matter are doing SIG_IGN so that seems like the one we
want to standardize on. In particular I think it's just fine that
we allow COPY TO/FROM PROGRAM children to inherit SIG_IGN for these.
We don't want parts of the system to freeze up if you failed to
dissociate the postmaster from your terminal, and that consideration
applies to the whole process tree not just parts of it.

I'm also pretty strongly tempted to make the postmaster code be

+#ifdef SIGTTIN
pqsignal(SIGTTIN, SIG_IGN); /* ignored */
+#endif
+#ifdef SIGTTOU
pqsignal(SIGTTOU, SIG_IGN); /* ignored */
+#endif

and then remove these lines in win32_port.h:

#define SIGTTIN 21
#define SIGTTOU 22 /* Same as SIGABRT -- no problem, I hope */

I'm not sure where anybody got the idea that SIG_IGN'ing SIGABRT
might be a sane thing to do.

Given the lack of complaints, there's probably no need for back-patch,
but that's what I'd propose in HEAD to make this saner.

regards, tom lane