Fix NULL pointer access in logical replication workers

petere · petere · commit 70c56a014e18 · 2017-01-23T12:33:27.000-05:00
From: Petr Jelinek &lt;pjmodos@pjmodos.net&gt;
diff --git 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
@@ -1219,45 +1219,45 @@ 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);
 		proc_exit(0);
 	}
 
 	/*
-	 * 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);
 		proc_exit(0);
 	}
 
 	/*
-	 * 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);
