Fix NULL pointer access in logical replication workers

From: Petr Jelinek <pjmodos@pjmodos.net>

2 files changed, 26 insertions, 15 deletions

diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index b5240dcedeb..18919724af4 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 7d86736444b..3ee9cc12df0 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);