From 00b1e9b947ca6c3d21a484cb9e7c7b3fa345d95e Mon Sep 17 00:00:00 2001
From: Dimitri Fontaine <dim@tapoueh.org>
Date: Tue, 28 Jul 2026 05:36:57 +0200
Subject: [PATCH] Fix #1166: distinguish a zombie postmaster from a live one in
 get_pgpid()

get_pgpid() probed liveness with a bare kill(pid, 0) == 0. A process that
has exited but not yet been reaped (a zombie) still holds a process-table
entry, so kill(pid, 0) reports it alive even though it will never do
anything again -- e.g. when Postgres is SIGKILLed mid-startup, before it
gets to write PM_STATUS_READY.

get_pgpid() is read from five different processes: the postgres controller
sub-process (the actual parent, since it fork()s + execv()s postgres
directly in service_postgres_start()), the keeper/node-active process, the
monitor reporting service, and one-shot CLI commands -- all sharing the
same on-disk postmaster.pid file. Because every one of them reaches the
same false alive conclusion from the same buggy probe, none of them ever
decides Postgres needs restarting: the controller sees nothing to do, the
keeper waits forever for a ready status a dead process will never write,
and the monitor never gets a heartbeat.

pid_is_alive() resolves this with waitpid(pid, WNOHANG) first: from the
controller (the true parent), this reaps the zombie right there and
reports not-alive, or reports 0 when the child is genuinely still
starting up (slow-startup behavior unchanged). From every other caller,
waitpid() on a pid that isn't their own child returns -1/ECHILD
immediately, falling back to the historical kill(pid, 0) probe -- no
behavior change there.

Verified with a standalone harness exercising the exact failure mode
(fork + exit without reaping): kill(pid, 0) reports the zombie alive as
expected (reproducing the bug), pid_is_alive() correctly reports it dead,
including on a repeated call after the process has been reaped. Also
verified end to end: clean build, docker-check (citus_indent) and
banned.h.sh both clean, and a full Docker pgaftest run of
basic_operation.pgaf (27/27) and timeline_fork_report_lsn_deadlock.pgaf
(6/6) -- both exercise get_pgpid() heavily across ordinary start/stop and
failover paths with no regression.
---
 src/bin/common/pgsetup.c | 50 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/src/bin/common/pgsetup.c b/src/bin/common/pgsetup.c
index 67217a8ef..175f02072 100644
--- a/src/bin/common/pgsetup.c
+++ b/src/bin/common/pgsetup.c
@@ -9,12 +9,14 @@
  *
  */
 
+#include <errno.h>
 #include <inttypes.h>
 #include <pwd.h>
 #include <signal.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
 #include <time.h>
 #include <unistd.h>
 
@@ -433,6 +435,52 @@ pg_setup_init(PostgresSetup *pgSetup,
 }
 
 
+/*
+ * pid_is_alive tells whether pid refers to a genuinely running process.
+ *
+ * A bare kill(pid, 0) cannot tell a zombie apart from a live process: a
+ * process that already exited but hasn't been reaped yet still holds a
+ * process-table entry, so the kernel reports it as "alive" even though it
+ * will never do anything again. When pid is one of our own children (as
+ * Postgres is for the postgres controller sub-process, which forks() and
+ * execv()s the postgres binary directly in service_postgres_start()),
+ * waitpid(pid, WNOHANG) resolves the ambiguity precisely: it reaps the
+ * zombie right here and reports "not alive", or reports that the child is
+ * still genuinely running.
+ *
+ * get_pgpid() is also called from processes that are not the postmaster's
+ * parent -- the keeper/node-active process, the monitor reporting service,
+ * one-shot CLI commands -- all reading the same on-disk postmaster.pid
+ * file. For those, waitpid() on a pid that isn't their own child returns
+ * -1/ECHILD immediately, and we fall back to the historical kill(pid, 0)
+ * probe, unchanged.
+ */
+static bool
+pid_is_alive(pid_t pid)
+{
+	int status;
+	pid_t ret;
+
+	do {
+		ret = waitpid(pid, &status, WNOHANG);
+	} while (ret == -1 && errno == EINTR);
+
+	if (ret == pid)
+	{
+		/* our own child, and it just exited: reaped, no longer alive */
+		return false;
+	}
+	else if (ret == 0)
+	{
+		/* our own child, still running */
+		return true;
+	}
+
+	/* not our child (ECHILD), or some other waitpid() failure */
+	return kill(pid, 0) == 0;
+}
+
+
 /*
  * Read the first line of the PGDATA/postmaster.pid file to get Postgres PID.
  */
@@ -494,7 +542,7 @@ get_pgpid(PostgresSetup *pgSetup, bool pgIsNotRunningIsOk)
 	}
 	else if (pid > 0 && pid <= INT_MAX)
 	{
-		if (kill(pid, 0) == 0)
+		if (pid_is_alive(pid))
 		{
 			pgSetup->pidFile.pid = pid;
 			return true;