From 0502d9ae46188ca6670d8ba81a2633dfc05f7967 Mon Sep 17 00:00:00 2001 From: Ayush Tiwari Date: Sun, 30 Aug 2026 12:57:21 +0000 Subject: [PATCH v1 2/2] Avoid deadlocks with DROP TABLESPACE DROP TABLESPACE now takes an AccessExclusiveLock on the tablespace before checking dependencies. Commands that update pg_tablespace without first locking the tablespace can hold its catalog tuple while DROP holds the object lock. If that transaction then records a tablespace dependency, the two sessions deadlock. Acquire AccessShareLock on the tablespace before changing its name or options. Do likewise for the internal ACL and owner updates performed by DROP OWNED and REASSIGN OWNED, rechecking the pg_shdepend tuple after any lock wait. Direct GRANT, REVOKE, and ALTER OWNER paths already acquire an object lock. Add isolation coverage for all four previously unlocked paths. --- src/backend/catalog/pg_shdepend.c | 37 ++++++++++++-- src/backend/commands/tablespace.c | 11 ++++- .../tablespace-dependency-locking.out | 48 +++++++++++++++++++ .../specs/tablespace-dependency-locking.spec | 34 ++++++++++++- 4 files changed, 122 insertions(+), 8 deletions(-) diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c index f70bffd527f..96ef6abf9dd 100644 --- a/src/backend/catalog/pg_shdepend.c +++ b/src/backend/catalog/pg_shdepend.c @@ -1199,15 +1199,14 @@ classIdGetDbId(Oid classId) /* * shdepLockAndCheckObject * - * Lock the object that we are about to record a dependency on. - * After it's locked, verify that it hasn't been dropped while we - * weren't looking. If the object has been dropped, this function - * does not return! + * Acquire an AccessShareLock on a shared object and verify that it still + * exists. This is used when recording a dependency or performing another + * operation that must protect the object against a concurrent DROP. */ void shdepLockAndCheckObject(Oid classId, Oid objectId) { - /* AccessShareLock should be OK, since we are not modifying the object */ + /* AccessShareLock is sufficient to prevent a concurrent DROP. */ LockSharedObject(classId, objectId, 0, AccessShareLock); switch (classId) @@ -1450,6 +1449,20 @@ shdepDropOwned(List *roleids, DropBehavior behavior) */ if (sdepForm->classid != AuthMemRelationId) { + /* Lock tablespaces before updating their catalog tuple. */ + if (sdepForm->classid == TableSpaceRelationId) + { + LockSharedObject(sdepForm->classid, + sdepForm->objid, 0, + AccessShareLock); + if (!systable_recheck_tuple(scan, tuple)) + { + UnlockSharedObject(sdepForm->classid, + sdepForm->objid, 0, + AccessShareLock); + break; + } + } RemoveRoleFromObjectACL(roleid, sdepForm->classid, sdepForm->objid); @@ -1605,6 +1618,20 @@ shdepReassignOwned(List *roleids, Oid newrole) switch (sdepForm->deptype) { case SHARED_DEPENDENCY_OWNER: + /* Lock tablespaces before updating their catalog tuple. */ + if (sdepForm->classid == TableSpaceRelationId) + { + LockSharedObject(sdepForm->classid, + sdepForm->objid, 0, + AccessShareLock); + if (!systable_recheck_tuple(scan, tuple)) + { + UnlockSharedObject(sdepForm->classid, + sdepForm->objid, 0, + AccessShareLock); + break; + } + } shdepReassignOwned_Owner(sdepForm, newrole); break; case SHARED_DEPENDENCY_INITACL: diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c index 99f6ecf2fd0..e01fb2db913 100644 --- a/src/backend/commands/tablespace.c +++ b/src/backend/commands/tablespace.c @@ -972,6 +972,9 @@ RenameTableSpace(const char *oldname, const char *newname) table_endscan(scan); + /* Lock the tablespace before updating its catalog tuple. */ + shdepLockAndCheckObject(TableSpaceRelationId, tspId); + /* Must be owner */ if (!object_ownercheck(TableSpaceRelationId, tspId, GetUserId())) aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLESPACE, oldname); @@ -1061,7 +1064,12 @@ AlterTableSpaceOptions(AlterTableSpaceOptionsStmt *stmt) errmsg("tablespace \"%s\" does not exist", stmt->tablespacename))); + tup = heap_copytuple(tup); tablespaceoid = ((Form_pg_tablespace) GETSTRUCT(tup))->oid; + table_endscan(scandesc); + + /* Lock the tablespace before updating its catalog tuple. */ + shdepLockAndCheckObject(TableSpaceRelationId, tablespaceoid); /* Must be owner of the existing object */ if (!object_ownercheck(TableSpaceRelationId, tablespaceoid, GetUserId())) @@ -1093,9 +1101,8 @@ AlterTableSpaceOptions(AlterTableSpaceOptionsStmt *stmt) InvokeObjectPostAlterHook(TableSpaceRelationId, tablespaceoid, 0); heap_freetuple(newtuple); + heap_freetuple(tup); - /* Conclude heap scan. */ - table_endscan(scandesc); table_close(rel, NoLock); return tablespaceoid; diff --git a/src/test/isolation/expected/tablespace-dependency-locking.out b/src/test/isolation/expected/tablespace-dependency-locking.out index 74d2b718ab2..2a6a0530584 100644 --- a/src/test/isolation/expected/tablespace-dependency-locking.out +++ b/src/test/isolation/expected/tablespace-dependency-locking.out @@ -34,3 +34,51 @@ step s1_rollback: ROLLBACK; step s2_drop_tablespace: <... completed> s3: NOTICE: referenced tablespace was concurrently dropped step s3_create_table_in_dropped_tablespace: <... completed> + +starting permutation: s1_begin s1_alter_tablespace s2_drop_tablespace s1_create_table_in_tablespace s1_commit +step s1_begin: BEGIN; +step s1_alter_tablespace: + ALTER TABLESPACE regress_dependency_tablespace + SET (random_page_cost = 1.1); + +step s2_drop_tablespace: DROP TABLESPACE regress_dependency_tablespace; +step s1_create_table_in_tablespace: + CREATE TABLE tbl_tablespace (a int) PARTITION BY RANGE (a) + TABLESPACE regress_dependency_tablespace; + +step s1_commit: COMMIT; +step s2_drop_tablespace: <... completed> +ERROR: tablespace "regress_dependency_tablespace" cannot be dropped because some objects depend on it + +starting permutation: s1_begin s1_rename_tablespace s2_drop_tablespace s1_create_table_in_renamed_tablespace s1_commit +step s1_begin: BEGIN; +step s1_rename_tablespace: ALTER TABLESPACE regress_dependency_tablespace RENAME TO regress_dependency_tablespace_renamed; +step s2_drop_tablespace: DROP TABLESPACE regress_dependency_tablespace; +step s1_create_table_in_renamed_tablespace: CREATE TABLE tbl_tablespace (a int) PARTITION BY RANGE (a) TABLESPACE regress_dependency_tablespace_renamed; +step s1_commit: COMMIT; +step s2_drop_tablespace: <... completed> +ERROR: tablespace "regress_dependency_tablespace" cannot be dropped because some objects depend on it + +starting permutation: s1_begin s1_reassign_owned s2_drop_tablespace s1_create_table_in_tablespace s1_commit +step s1_begin: BEGIN; +step s1_reassign_owned: REASSIGN OWNED BY regress_ts_owner TO CURRENT_USER; +step s2_drop_tablespace: DROP TABLESPACE regress_dependency_tablespace; +step s1_create_table_in_tablespace: + CREATE TABLE tbl_tablespace (a int) PARTITION BY RANGE (a) + TABLESPACE regress_dependency_tablespace; + +step s1_commit: COMMIT; +step s2_drop_tablespace: <... completed> +ERROR: tablespace "regress_dependency_tablespace" cannot be dropped because some objects depend on it + +starting permutation: s1_begin s1_drop_owned s2_drop_tablespace s1_create_table_in_tablespace s1_commit +step s1_begin: BEGIN; +step s1_drop_owned: DROP OWNED BY regress_ts_grantee; +step s2_drop_tablespace: DROP TABLESPACE regress_dependency_tablespace; +step s1_create_table_in_tablespace: + CREATE TABLE tbl_tablespace (a int) PARTITION BY RANGE (a) + TABLESPACE regress_dependency_tablespace; + +step s1_commit: COMMIT; +step s2_drop_tablespace: <... completed> +ERROR: tablespace "regress_dependency_tablespace" cannot be dropped because some objects depend on it diff --git a/src/test/isolation/specs/tablespace-dependency-locking.spec b/src/test/isolation/specs/tablespace-dependency-locking.spec index fa0b00aeb99..23de346c5f7 100644 --- a/src/test/isolation/specs/tablespace-dependency-locking.spec +++ b/src/test/isolation/specs/tablespace-dependency-locking.spec @@ -4,11 +4,20 @@ setup { SET allow_in_place_tablespaces = true; + CREATE ROLE regress_ts_owner; + CREATE ROLE regress_ts_grantee; } setup { - CREATE TABLESPACE regress_dependency_tablespace LOCATION ''; + CREATE TABLESPACE regress_dependency_tablespace + OWNER regress_ts_owner LOCATION ''; +} + +setup +{ + GRANT CREATE ON TABLESPACE regress_dependency_tablespace + TO regress_ts_grantee; } teardown @@ -29,6 +38,10 @@ step "s1_alter_tablespace" ALTER TABLESPACE regress_dependency_tablespace SET (random_page_cost = 1.1); } +step "s1_rename_tablespace" { ALTER TABLESPACE regress_dependency_tablespace RENAME TO regress_dependency_tablespace_renamed; } +step "s1_reassign_owned" { REASSIGN OWNED BY regress_ts_owner TO CURRENT_USER; } +step "s1_drop_owned" { DROP OWNED BY regress_ts_grantee; } +step "s1_create_table_in_renamed_tablespace" { CREATE TABLE tbl_tablespace (a int) PARTITION BY RANGE (a) TABLESPACE regress_dependency_tablespace_renamed; } step "s1_commit" { COMMIT; } step "s1_rollback" { ROLLBACK; } step "s1_drop_table" { DROP TABLE tbl_tablespace; } @@ -38,6 +51,19 @@ teardown { SET client_min_messages = warning; DROP TABLE IF EXISTS tbl_tablespace; + DROP OWNED BY regress_ts_grantee; + REASSIGN OWNED BY regress_ts_owner TO CURRENT_USER; + DO $$ + BEGIN + IF EXISTS (SELECT FROM pg_tablespace + WHERE spcname = 'regress_dependency_tablespace_renamed') THEN + ALTER TABLESPACE regress_dependency_tablespace_renamed + RENAME TO regress_dependency_tablespace; + END IF; + END + $$; + DROP ROLE IF EXISTS regress_ts_owner; + DROP ROLE IF EXISTS regress_ts_grantee; } session "s2" @@ -64,3 +90,9 @@ permutation "s1_begin" "s1_create_table_in_tablespace" "s2_drop_tablespace" "s1_ # drop tablespace - create table; ALTER makes DROP wait while deleting the # catalog tuple, after DROP has checked for dependencies permutation "s1_begin" "s1_alter_tablespace" "s2_drop_tablespace" "s3_create_table_in_dropped_tablespace" "s1_rollback" + +# pg_tablespace updates must lock the tablespace before the catalog tuple +permutation "s1_begin" "s1_alter_tablespace" "s2_drop_tablespace" "s1_create_table_in_tablespace" "s1_commit" +permutation "s1_begin" "s1_rename_tablespace" "s2_drop_tablespace" "s1_create_table_in_renamed_tablespace" "s1_commit" +permutation "s1_begin" "s1_reassign_owned" "s2_drop_tablespace" "s1_create_table_in_tablespace" "s1_commit" +permutation "s1_begin" "s1_drop_owned" "s2_drop_tablespace" "s1_create_table_in_tablespace" "s1_commit" -- 2.43.0