At Thu, 14 May 2020 01:03:48 +0000, "Bossart, Nathan" <bossartn@amazon.com> wrote in
> I think I've discovered a problem with 850196b6. The following steps
> can be used to trigger a segfault:
>
> # wal_level = logical
> psql postgres -c "create database testdb;"
> psql testdb -c "select pg_create_logical_replication_slot('slot', 'test_decoding');"
> psql "dbname=postgres replication=database" -c "START_REPLICATION SLOT slot LOGICAL 0/0;"
>
> From a quick glance, I think the problem starts in
> StartLogicalReplication() in walsender.c. The call to
> CreateDecodingContext() may ERROR before xlogreader is initialized in
> the next line, so the subsequent call to WalSndErrorCleanup()
> segfaults when it attempts to access xlogreader.
Good catch! That's not only for CreateDecodingContet. That happens
everywhere in the query loop in PostgresMain() until logreader is
initialized. So that also happens, for example, by starting logical
replication using invalidated slot. Checking xlogreader != NULL in
WalSndErrorCleanup is sufficient. It doesn't make actual difference,
but the attached explicitly initialize the pointer with NULL.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From 5421f5b0e63abfe18b6f09d75c44697cdac699f3 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyoga.ntt@gmail.com>
Date: Thu, 14 May 2020 13:37:50 +0900
Subject: [PATCH] Don't check uninitialized xlog reader state
WanSndErrorCleanup may be called before initialization of the variable
xlogreader and crashes in that case. Let the function not to examine
the xlogreader state if not yet initialized.
---
src/backend/replication/walsender.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 3367aa98f8..661fa12a94 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -136,7 +136,7 @@ bool wake_wal_senders = false;
* routines.
*/
static XLogReaderState fake_xlogreader;
-static XLogReaderState *xlogreader;
+static XLogReaderState *xlogreader = NULL;
/*
* These variables keep track of the state of the timeline we're currently
@@ -315,7 +315,7 @@ WalSndErrorCleanup(void)
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- if (xlogreader->seg.ws_file >= 0)
+ if (xlogreader != NULL && xlogreader->seg.ws_file >= 0)
wal_segment_close(xlogreader);
if (MyReplicationSlot != NULL)
--
2.18.2
