Detach postmaster process from pg_ctl's session at server startup.

pg_ctl is supposed to daemonize the postmaster process, so that it's not
affected by signals to the launching process group.  Before this patch, if
you had a shell script that used "pg_ctl start", and you interrupted the
shell script after postmaster had been launched, postmaster was also
killed.  To fix, call setsid() after forking the postmaster process.

Long time ago, we had a 'silent_mode' option, which daemonized the
postmaster process by calling setsid(), but that was removed back in 2011
(commit f7ea6beaf4).  We discussed bringing that back in some form, but
pg_ctl is the documented way of launching postmaster to the background, so
putting the setsid() call in pg_ctl itself seems appropriate.

Just putting postmaster in a separate session would change the behavior
when you interrupt "pg_ctl -w start", e.g. with CTRL-C, while it's waiting
for postmaster to start.  The historical behavior has been that
interrupting pg_ctl aborts the server launch, which is handy if the server
is stuck in recovery, for example, and won't fully start up.  To keep that
behavior, install a signal handler in pg_ctl, to explicitly kill
postmaster, if pg_ctl is interrupted while it's waiting for the server to
start up.  This isn't 100% watertight, there is a small window after
forking the postmaster process, where the signal handler doesn't know the
postmaster's PID yet, but seems good enough.

Arguably this is a long-standing bug, but I refrained from back-batching,
out of fear of breaking someone's scripts that depended on the old
behavior.

Reviewed by Tom Lane.  Report and original patch by Paul Guo, with
feedback from Michael Paquier.

Discussion: https://www.postgresql.org/message-id/CAEET0ZH5Bf7dhZB3mYy8zZQttJrdZg_0Wwaj0o1PuuBny1JkEw%40mail.gmail.com

diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index aadc70113de14ae020349b75fa0e149f3415cc60..24caa6fca5a781b42a797cc4633027113c72d371 100644 (file)
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -103,12 +103,13 @@ static char backup_file[MAXPGPATH];
 static char promote_file[MAXPGPATH];
 static char logrotate_file[MAXPGPATH];
 
+static volatile pgpid_t postmasterPID = -1;
+
 #ifdef WIN32
 static DWORD pgctl_start_type = SERVICE_AUTO_START;
 static SERVICE_STATUS status;
 static SERVICE_STATUS_HANDLE hStatus = (SERVICE_STATUS_HANDLE) 0;
 static HANDLE shutdownHandles[2];
-static pid_t postmasterPID = -1;
 
 #define shutdownEvent    shutdownHandles[0]
 #define postmasterProcess shutdownHandles[1]
@@ -471,6 +472,20 @@ start_postmaster(void)
 
        /* fork succeeded, in child */
 
+       /*
+        * If possible, detach the postmaster process from the launching process
+        * group and make it a group leader, so that it doesn't get signaled along
+        * with the current group that launched it.
+        */
+#ifdef HAVE_SETSID
+       if (setsid() < 0)
+       {
+               write_stderr(_("%s: could not start server due to setsid() failure: %s\n"),
+                                        progname, strerror(errno));
+               exit(1);
+       }
+#endif
+
        /*
         * Since there might be quotes to handle here, it is easier simply to pass
         * everything to a shell to process them.  Use exec so that the postmaster
@@ -719,6 +734,30 @@ read_post_opts(void)
        }
 }
 
+/*
+ * SIGINT signal handler used while waiting for postmaster to start up.
+ * Forwards the SIGINT to the postmaster process, asking it to shut down,
+ * before terminating pg_ctl itself. This way, if the user hits CTRL-C while
+ * waiting for the server to start up, the server launch is aborted.
+ */
+static void
+trap_sigint_during_startup(int sig)
+{
+       if (postmasterPID != -1)
+       {
+               if (kill(postmasterPID, SIGINT) != 0)
+                       write_stderr(_("%s: could not send stop signal (PID: %ld): %s\n"),
+                                                progname, (pgpid_t) postmasterPID, strerror(errno));
+       }
+
+       /*
+        * Clear the signal handler, and send the signal again, to terminate the
+        * process as normal.
+        */
+       pqsignal(SIGINT, SIG_DFL);
+       raise(SIGINT);
+}
+
 static char *
 find_other_exec_or_die(const char *argv0, const char *target, const char *versionstr)
 {
@@ -827,6 +866,17 @@ do_start(void)
 
        if (do_wait)
        {
+               /*
+                * If the user interrupts the startup (e.g. with CTRL-C), we'd like to
+                * abort the server launch.  Install a signal handler that will
+                * forward SIGINT to the postmaster process, while we wait.
+                *
+                * (We don't bother to reset the signal handler after the launch, as
+                * we're about to exit, anyway.)
+                */
+               postmasterPID = pm_pid;
+               pqsignal(SIGINT, trap_sigint_during_startup);
+
                print_msg(_("waiting for server to start..."));
 
                switch (wait_for_postmaster(pm_pid, false))