public inbox for [email protected]
help / color / mirror / Atom feedFrom: SATYANARAYANA NARLAPURAM <[email protected]>
To: Chao Li <[email protected]>
To: Tom Lane <[email protected]>
Cc: Japin Li <[email protected]>
Cc: Jack Bonatakis <[email protected]>
Cc: [email protected]
Subject: Re: Fix a server crash problem from pg_get_database_ddl
Date: Wed, 22 Apr 2026 23:47:39 -0700
Message-ID: <CAHg+QDcNyJ94cCD+9ZRfz==hDnghjE5BaR4+BiSWXt82hpgDtA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<SY7PR01MB109214566B069E9C9084590FEB6232@SY7PR01MB10921.ausprd01.prod.outlook.com>
<[email protected]>
Hi,
Adding Tom to the thread explicitly to seek his opinion.
On Wed, Apr 15, 2026 at 6:36 PM Chao Li <[email protected]> wrote:
>
>
> > On Apr 16, 2026, at 09:23, Japin Li <[email protected]> wrote:
> >
> > On Wed, 15 Apr 2026 at 20:44, "Jack Bonatakis" <[email protected]> wrote:
> >> I have reproduced this error against the current master:
> >>
> >> ```
> >> CREATE TABLESPACE ts1 LOCATION '/workspace/tablespaces/pg_bug_ts1';
> >> CREATE DATABASE db1 TABLESPACE ts1;
> >> DELETE FROM pg_tablespace WHERE spcname = 'ts1';
> >> SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
> >>
> >> server closed the connection unexpectedly
> >> This probably means the server terminated abnormally
> >> before or while processing the request.
> >> The connection to the server was lost. Attempting reset: Failed.
> >> ```
> >> Backend logs show:
> >>
> >> ```
> >> [1] LOG: client backend (PID 15420) was terminated by signal 11:
> Segmentation fault
> >> [1] DETAIL: Failed process was running: SELECT * FROM
> pg_get_database_ddl('db1'::regdatabase);
> >> [1] LOG: terminating any other active server processes
> >> ```
> >> After applying the patch:
> >>
> >> ```
> >> SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
> >> ERROR: tablespace with OID 16393 does not exist
> >> HINT: To recover, try ALTER DATABASE ... SET TABLESPACE ... to a valid
> tablespace.
> >> ```
> >> and backend logs show:
> >>
> >> ```
> >> [56] ERROR: tablespace with OID 16393 does not exist
> >> [56] HINT: To recover, try ALTER DATABASE ... SET TABLESPACE ... to a
> valid tablespace.
> >> [56] STATEMENT: SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
> >> ```
> >> All tests pass.
> >>
> >> The only note I'd have on the code change is that there is no
> accompanying test. It seems like a TAP test would be
> >> reasonable, but I am quite new and will defer to whether you think
> that's the right call or even necessary.
> >>
> >> Jack
> >
> > This seems similar to [1]. Could you please confirm?
> >
> > [1]
> https://www.postgresql.org/message-id/CAJTYsWXcd324VELk%3D9KdsfTsua9So3Yexqv7N3B23h9zAUD40g%40mail.g...
> .
> >
> > --
> > Regards,
> > Japin Li
> > ChengDu WenWu Information Technology Co., Ltd.
> >
> >
>
> Thanks for printing out that. Yes, they are similar.
>
> I agree with what Tom said in [2]:
> ```
> This is not a bug. This is a superuser intentionally breaking
> the system by corrupting the catalogs. There are any number
> of ways to cause trouble with ill-advised manual updates to a
> catalog table. Try, eg, "DELETE FROM pg_proc" (... but not in
> a database you care about).
> ```
>
> So, let me take back this patch.
>
> [2] https://www.postgresql.org/message-id/[email protected]
In this case, it is a very corner case but not something superuser
intentionally breaks.
For example, a concurrent tablespace drop + database ddl to assign a
different tablespace or default.
We aren't acquiring Access Share lock on the DB in this function
(intentional) so it is a good practice
to do the null checks. Of course, it makes more sense to add this comment
while doing a code review.
I will let Tom and others chime in with their thoughts on fixing this.
Attached an injection point test to show the race. Not intended to commit.
Thanks,
Satya
Attachments:
[application/octet-stream] 0002-Test-pg_get_database_ddl-race-with-concurrent-tablespace-drop.patch (5.1K, 3-0002-Test-pg_get_database_ddl-race-with-concurrent-tablespace-drop.patch)
download | inline diff:
diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c
index d83cda33..b57e7d27 100644
--- a/src/backend/utils/adt/ddlutils.c
+++ b/src/backend/utils/adt/ddlutils.c
@@ -35,6 +35,7 @@
#include "utils/acl.h"
#include "utils/array.h"
#include "utils/builtins.h"
+#include "utils/injection_point.h"
#include "utils/datetime.h"
#include "utils/fmgroids.h"
#include "utils/guc.h"
@@ -984,9 +985,13 @@ pg_get_database_ddl_internal(Oid dbid, bool pretty,
/* TABLESPACE */
if (!no_tablespace && OidIsValid(dbform->dattablespace))
{
- char *spcname = get_tablespace_name(dbform->dattablespace);
+ char *spcname;
- if (pg_strcasecmp(spcname, "pg_default") != 0)
+ INJECTION_POINT("pg_get_database_ddl-before-get_tablespace_name", NULL);
+ spcname = get_tablespace_name(dbform->dattablespace);
+
+ if (spcname != NULL &&
+ pg_strcasecmp(spcname, "pg_default") != 0)
append_ddl_option(&buf, pretty, 4, "TABLESPACE = %s",
quote_identifier(spcname));
}
diff --git a/src/test/modules/test_misc/t/012_ddlutils_tablespace_race.pl b/src/test/modules/test_misc/t/012_ddlutils_tablespace_race.pl
new file mode 100644
index 00000000..76d31afb
--- /dev/null
+++ b/src/test/modules/test_misc/t/012_ddlutils_tablespace_race.pl
@@ -0,0 +1,111 @@
+
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test for race condition in pg_get_database_ddl() when a tablespace
+# is dropped concurrently.
+#
+# pg_get_database_ddl() reads the database's dattablespace OID from the
+# syscache, then later calls get_tablespace_name() which does a fresh
+# catalog scan. If the tablespace is dropped between these two operations,
+# get_tablespace_name() returns NULL and the unpatched code passes NULL
+# to pg_strcasecmp(), causing a SIGSEGV.
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+plan skip_all => 'Injection points not supported by this build'
+ unless $ENV{enable_injection_points} eq 'yes';
+
+# Node initialization
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init();
+$node->append_conf('postgresql.conf',
+ "shared_preload_libraries = 'injection_points'");
+$node->start();
+
+# Check if the extension injection_points is available
+plan skip_all => 'Extension injection_points not installed'
+ unless $node->check_extension('injection_points');
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# Create tablespace and database using it
+$node->safe_psql('postgres', q[
+SET allow_in_place_tablespaces = on;
+CREATE TABLESPACE race_ts LOCATION '';
+]);
+$node->safe_psql('postgres',
+ "CREATE DATABASE race_db TABLESPACE race_ts;");
+
+# Verify setup
+my $result = $node->safe_psql('postgres',
+ "SELECT spcname FROM pg_database d JOIN pg_tablespace t "
+ . "ON d.dattablespace = t.oid WHERE datname = 'race_db';");
+is($result, 'race_ts', 'race_db is on race_ts tablespace');
+
+############################################################################
+note('Test: pg_get_database_ddl with concurrent tablespace drop');
+
+# Attach injection point that pauses before get_tablespace_name()
+$node->safe_psql('postgres',
+ "SELECT injection_points_attach("
+ . "'pg_get_database_ddl-before-get_tablespace_name', 'wait');");
+
+# Session A: call pg_get_database_ddl - will block at injection point
+my $session_a = $node->background_psql('postgres');
+
+$session_a->query_until(
+ qr//, q[
+SELECT pg_get_database_ddl('race_db'::regdatabase);
+\g
+]);
+
+# Wait for Session A to reach the injection point
+$node->wait_for_event('client backend',
+ 'pg_get_database_ddl-before-get_tablespace_name');
+
+note('Session A is paused before get_tablespace_name()');
+
+# Session B: move database off the tablespace and drop it
+$node->safe_psql('postgres',
+ "ALTER DATABASE race_db SET TABLESPACE pg_default;");
+$node->safe_psql('postgres', "DROP TABLESPACE race_ts;");
+
+# Verify the tablespace is gone
+$result = $node->safe_psql('postgres',
+ "SELECT count(*) FROM pg_tablespace WHERE spcname = 'race_ts';");
+is($result, '0', 'tablespace race_ts has been dropped');
+
+note('Waking up Session A - get_tablespace_name() will return NULL');
+
+# Wake up Session A - it will now call get_tablespace_name() with a
+# stale OID that no longer exists. Without the NULL check fix, this
+# crashes with SIGSEGV in pg_strcasecmp(NULL, "pg_default").
+$node->safe_psql('postgres',
+ "SELECT injection_points_wakeup("
+ . "'pg_get_database_ddl-before-get_tablespace_name');");
+
+# Collect result from Session A
+my $output = $session_a->query_safe(q[\g]);
+$session_a->quit();
+
+# The DDL should succeed and NOT include TABLESPACE (since the database
+# is now on pg_default)
+like($output, qr/CREATE DATABASE race_db/,
+ 'pg_get_database_ddl completed without crash');
+unlike($output, qr/TABLESPACE/,
+ 'DDL output does not include dropped tablespace');
+
+# Detach injection point
+$node->safe_psql('postgres',
+ "SELECT injection_points_detach("
+ . "'pg_get_database_ddl-before-get_tablespace_name');");
+
+# Clean up
+$node->safe_psql('postgres', "DROP DATABASE race_db;");
+
+done_testing();
view thread (10+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Fix a server crash problem from pg_get_database_ddl
In-Reply-To: <CAHg+QDcNyJ94cCD+9ZRfz==hDnghjE5BaR4+BiSWXt82hpgDtA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox