diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c index 753afb88453..f7ec9aa2f3c 100644 --- a/src/backend/catalog/pg_shdepend.c +++ b/src/backend/catalog/pg_shdepend.c @@ -1202,15 +1202,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) @@ -1453,6 +1452,23 @@ 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); @@ -1608,6 +1624,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 113b4807315..8bf851519c0 100644 --- a/src/backend/commands/tablespace.c +++ b/src/backend/commands/tablespace.c @@ -70,6 +70,9 @@ #include "miscadmin.h" #include "postmaster/bgwriter.h" #include "storage/fd.h" +#include "storage/lmgr.h" +#include "storage/lwlock.h" +#include "storage/procsignal.h" #include "storage/standby.h" #include "utils/acl.h" #include "utils/builtins.h" @@ -449,6 +452,10 @@ DropTableSpace(DropTableSpaceStmt *stmt) aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLESPACE, tablespacename); + /* Prevent new shared dependencies while we drop the tablespace. */ + LockSharedObject(TableSpaceRelationId, tablespaceoid, 0, + AccessExclusiveLock); + /* Check for pg_shdepend entries depending on this tablespace */ if (checkSharedDependencies(TableSpaceRelationId, tablespaceoid, &detail, &detail_log)) @@ -959,6 +966,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); @@ -1042,7 +1052,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())) @@ -1074,9 +1089,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 new file mode 100644 index 00000000000..2a6a0530584 --- /dev/null +++ b/src/test/isolation/expected/tablespace-dependency-locking.out @@ -0,0 +1,84 @@ +Parsed test spec with 3 sessions + +starting permutation: s1_begin s1_create_table_in_tablespace s2_drop_tablespace s1_commit s1_drop_table s1_drop_tablespace +step s1_begin: BEGIN; +step s1_create_table_in_tablespace: + CREATE TABLE tbl_tablespace (a int) PARTITION BY RANGE (a) + TABLESPACE regress_dependency_tablespace; + +step s2_drop_tablespace: DROP 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 +step s1_drop_table: DROP TABLE tbl_tablespace; +step s1_drop_tablespace: DROP TABLESPACE regress_dependency_tablespace; + +starting permutation: s1_begin s1_alter_tablespace s2_drop_tablespace s3_create_table_in_dropped_tablespace s1_rollback +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 s3_create_table_in_dropped_tablespace: + DO $$ + BEGIN + EXECUTE 'CREATE TABLE tbl_tablespace (a int) PARTITION BY RANGE (a) + TABLESPACE regress_dependency_tablespace'; + EXCEPTION WHEN undefined_object THEN + RAISE NOTICE 'referenced tablespace was concurrently dropped'; + END + $$; + +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/isolation_schedule b/src/test/isolation/isolation_schedule index d7cebb7c440..db479c35204 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -117,5 +117,6 @@ test: serializable-parallel-3 test: matview-write-skew test: lock-nowait test: ddl-dependency-locking +test: tablespace-dependency-locking test: pub-concurrent-drop test: drop-owned-grant diff --git a/src/test/isolation/specs/tablespace-dependency-locking.spec b/src/test/isolation/specs/tablespace-dependency-locking.spec new file mode 100644 index 00000000000..23de346c5f7 --- /dev/null +++ b/src/test/isolation/specs/tablespace-dependency-locking.spec @@ -0,0 +1,98 @@ +# Test that concurrent DROP TABLESPACE and CREATE TABLE do not leave behind +# references to a non-existent tablespace. + +setup +{ + SET allow_in_place_tablespaces = true; + CREATE ROLE regress_ts_owner; + CREATE ROLE regress_ts_grantee; +} + +setup +{ + CREATE TABLESPACE regress_dependency_tablespace + OWNER regress_ts_owner LOCATION ''; +} + +setup +{ + GRANT CREATE ON TABLESPACE regress_dependency_tablespace + TO regress_ts_grantee; +} + +teardown +{ + DROP TABLESPACE IF EXISTS regress_dependency_tablespace; +} + +session "s1" + +step "s1_begin" { BEGIN; } +step "s1_create_table_in_tablespace" +{ + CREATE TABLE tbl_tablespace (a int) PARTITION BY RANGE (a) + TABLESPACE regress_dependency_tablespace; +} +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; } +step "s1_drop_tablespace" { DROP TABLESPACE regress_dependency_tablespace; } + +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" + +step "s2_drop_tablespace" { DROP TABLESPACE regress_dependency_tablespace; } + +session "s3" + +step "s3_create_table_in_dropped_tablespace" +{ + DO $$ + BEGIN + EXECUTE 'CREATE TABLE tbl_tablespace (a int) PARTITION BY RANGE (a) + TABLESPACE regress_dependency_tablespace'; + EXCEPTION WHEN undefined_object THEN + RAISE NOTICE 'referenced tablespace was concurrently dropped'; + END + $$; +} + +# create table - drop tablespace +permutation "s1_begin" "s1_create_table_in_tablespace" "s2_drop_tablespace" "s1_commit" "s1_drop_table" "s1_drop_tablespace" + +# 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"