Fix walsender error cleanup code

In commit 850196b610d2 I (Álvaro) failed to handle the case of walsender
shutting down on an error before setting up its 'xlogreader' pointer;
the error handling code dereferences the pointer, causing a crash.
Fix by testing the pointer before trying to dereference it.

Kyotaro authored the code fix; I adopted Nathan's test case to be used
by the TAP tests and added the necessary PostgresNode change.

Reported-by: Nathan Bossart 
Author: Kyotaro Horiguchi 
Author: Álvaro Herrera 
Discussion: https://postgr.es/m/C04FC24E-903D-4423-B312-6910E4D846E5@amazon.com

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index a4ca8daea77643c4b24d5da456f9d30708ef3576..86847cbb54f54bf5ce0bcc5f0da86b053bb759b6 100644 (file)
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -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)

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 3f3a1d81f68e01b1f758ed6a7ad264568311833d..1407359aef66fa441b69acd80a88f248229c5a80 100644 (file)
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -1386,6 +1386,12 @@ the B parameter is also given.
 If B is set and this parameter is given, the scalar it references
 is set to true if the psql call times out.
 
+=item replication => B
+
+If set, add B to the conninfo string.
+Passing the literal value C results in a logical replication
+connection.
+
 =item extra_params => ['--single-transaction']
 
 If given, it must be an array reference containing additional parameters to B.
@@ -1414,10 +1420,17 @@ sub psql
 
    my $stdout            = $params{stdout};
    my $stderr            = $params{stderr};
+   my $replication       = $params{replication};
    my $timeout           = undef;
    my $timeout_exception = 'psql timed out';
-   my @psql_params =
-     ('psql', '-XAtq', '-d', $self->connstr($dbname), '-f', '-');
+   my @psql_params       = (
+       'psql',
+       '-XAtq',
+       '-d',
+       $self->connstr($dbname)
+         . (defined $replication ? " replication=$replication" : ""),
+       '-f',
+       '-');
 
    # If the caller wants an array and hasn't passed stdout/stderr
    # references, allocate temporary ones to capture them so we

diff --git a/src/test/recovery/t/006_logical_decoding.pl b/src/test/recovery/t/006_logical_decoding.pl
index d40a500ed4790559942114f8ff87339b6ab4b7a4..ee05535b1c20506fb42d00fec3cf5cff24195816 100644 (file)
--- a/src/test/recovery/t/006_logical_decoding.pl
+++ b/src/test/recovery/t/006_logical_decoding.pl
@@ -7,7 +7,7 @@ use strict;
 use warnings;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 12;
+use Test::More tests => 13;
 use Config;
 
 # Initialize master node
@@ -27,12 +27,21 @@ $node_master->safe_psql('postgres',
    qq[SELECT pg_create_logical_replication_slot('test_slot', 'test_decoding');]
 );
 
+# Cover walsender error shutdown code
+my ($result, $stdout, $stderr) = $node_master->psql(
+   'template1',
+   qq[START_REPLICATION SLOT test_slot LOGICAL 0/0],
+   replication => 'database');
+ok( $stderr =~
+     m/replication slot "test_slot" was not created in this database/,
+   "Logical decoding correctly fails to start");
+
 $node_master->safe_psql('postgres',
    qq[INSERT INTO decoding_test(x,y) SELECT s, s::text FROM generate_series(1,10) s;]
 );
 
 # Basic decoding works
-my ($result) = $node_master->safe_psql('postgres',
+$result = $node_master->safe_psql('postgres',
    qq[SELECT pg_logical_slot_get_changes('test_slot', NULL, NULL);]);
 is(scalar(my @foobar = split /^/m, $result),
    12, 'Decoding produced 12 rows inc BEGIN/COMMIT');