From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 23 Jan 2017 17:33:27 +0000 (-0500)
Subject: Fix NULL pointer access in logical replication workers
X-Git-Tag: REL_10_BETA1~1041
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=70c56a014e1813b5ab0f91581833bc7cb6c02958;p=postgresql

Fix NULL pointer access in logical replication workers

From: Petr Jelinek <pjmodos@pjmodos.net>
---

diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index b5240dcede..18919724af 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -349,10 +349,21 @@ logicalrep_worker_stop(Oid subid)
 
 		ResetLatch(&MyProc->procLatch);
 
-		/* Check if the worker has started. */
+		/* Check worker status. */
 		LWLockAcquire(LogicalRepWorkerLock, LW_SHARED);
-		worker = logicalrep_worker_find(subid);
-		if (!worker || worker->proc)
+
+		/*
+		 * Worker is no longer associated with subscription.  It must have
+		 * exited, nothing more for us to do.
+		 */
+		if (worker->subid == InvalidOid)
+		{
+			LWLockRelease(LogicalRepWorkerLock);
+			return;
+		}
+
+		/* Worker has assigned proc, so it has started. */
+		if (worker->proc)
 			break;
 	}
 
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 7d86736444..3ee9cc12df 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -1219,14 +1219,15 @@ reread_subscription(void)
 	newsub = GetSubscription(MyLogicalRepWorker->subid, true);
 
 	/*
-	 * Exit if connection string was changed. The launcher will start
-	 * new worker.
+	 * Exit if the subscription was removed.
+	 * This normally should not happen as the worker gets killed
+	 * during DROP SUBSCRIPTION.
 	 */
-	if (strcmp(newsub->conninfo, MySubscription->conninfo) != 0)
+	if (!newsub)
 	{
 		ereport(LOG,
 				(errmsg("logical replication worker for subscription \"%s\" will "
-						"restart because the connection information was changed",
+						"stop because the subscription was removed",
 						MySubscription->name)));
 
 		walrcv_disconnect(wrconn);
@@ -1234,14 +1235,14 @@ reread_subscription(void)
 	}
 
 	/*
-	 * Exit if publication list was changed. The launcher will start
+	 * Exit if connection string was changed. The launcher will start
 	 * new worker.
 	 */
-	if (!equal(newsub->publications, MySubscription->publications))
+	if (strcmp(newsub->conninfo, MySubscription->conninfo) != 0)
 	{
 		ereport(LOG,
 				(errmsg("logical replication worker for subscription \"%s\" will "
-						"restart because subscription's publications were changed",
+						"restart because the connection information was changed",
 						MySubscription->name)));
 
 		walrcv_disconnect(wrconn);
@@ -1249,15 +1250,14 @@ reread_subscription(void)
 	}
 
 	/*
-	 * Exit if the subscription was removed.
-	 * This normally should not happen as the worker gets killed
-	 * during DROP SUBSCRIPTION.
+	 * Exit if publication list was changed. The launcher will start
+	 * new worker.
 	 */
-	if (!newsub)
+	if (!equal(newsub->publications, MySubscription->publications))
 	{
 		ereport(LOG,
 				(errmsg("logical replication worker for subscription \"%s\" will "
-						"stop because the subscription was removed",
+						"restart because subscription's publications were changed",
 						MySubscription->name)));
 
 		walrcv_disconnect(wrconn);