Skip to content

Commit

Permalink
Avoid "you don't own a lock of type ExclusiveLock" in GRANT TABLESPACE.
Browse files Browse the repository at this point in the history
This WARNING appeared because SearchSysCacheLocked1() read
cc_relisshared before catcache initialization, when the field is false
unconditionally.  On the basis of reading false there, it constructed a
locktag as though pg_tablespace weren't relisshared.  Only shared
catalogs could be affected, and only GRANT TABLESPACE was affected in
practice.  SearchSysCacheLocked1() callers use one other shared-relation
syscache, DATABASEOID.  DATABASEOID is initialized by the end of
CheckMyDatabase(), making the problem unreachable for pg_database.

Back-patch to v13 (all supported versions).  This has no known impact
before v16, where ExecGrant_common() first appeared.  Earlier branches
avoid trouble by having a separate ExecGrant_Tablespace() that doesn't
use LOCKTAG_TUPLE.  However, leaving this unfixed in v15 could ensnare a
future back-patch of a SearchSysCacheLocked1() call.

Reported by Aya Iwata.

Discussion: https://postgr.es/m/OS7PR01MB11964507B5548245A7EE54E70EA212@OS7PR01MB11964.jpnprd01.prod.outlook.com
  • Loading branch information
nmisch authored and mrdrivingduck committed Nov 27, 2024
1 parent be05eb6 commit e8c78ed
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 5 deletions.
15 changes: 10 additions & 5 deletions src/backend/utils/cache/syscache.c
Original file line number Diff line number Diff line change
Expand Up @@ -1242,11 +1242,9 @@ HeapTuple
SearchSysCacheLocked1(int cacheId,
Datum key1)
{
CatCache *cache = SysCache[cacheId];
ItemPointerData tid;
LOCKTAG tag;
Oid dboid =
SysCache[cacheId]->cc_relisshared ? InvalidOid : MyDatabaseId;
Oid reloid = cacheinfo[cacheId].reloid;

/*----------
* Since inplace updates may happen just before our LockTuple(), we must
Expand Down Expand Up @@ -1298,8 +1296,15 @@ SearchSysCacheLocked1(int cacheId,

tid = tuple->t_self;
ReleaseSysCache(tuple);
/* like: LockTuple(rel, &tid, lockmode) */
SET_LOCKTAG_TUPLE(tag, dboid, reloid,

/*
* Do like LockTuple(rel, &tid, lockmode). While cc_relisshared won't
* change from one iteration to another, it may have been a temporary
* "false" until our first SearchSysCache1().
*/
SET_LOCKTAG_TUPLE(tag,
cache->cc_relisshared ? InvalidOid : MyDatabaseId,
cache->cc_reloid,
ItemPointerGetBlockNumber(&tid),
ItemPointerGetOffsetNumber(&tid));
(void) LockAcquire(&tag, lockmode, false, false);
Expand Down
5 changes: 5 additions & 0 deletions src/test/regress/expected/tablespace.out
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,11 @@ ALTER INDEX testschema.part_a_idx SET TABLESPACE pg_default;
-- Fail, not empty
DROP TABLESPACE regress_tblspace;
ERROR: tablespace "regress_tblspace" is not empty
-- Adequate cache initialization before GRANT
\c -
BEGIN;
GRANT ALL ON TABLESPACE regress_tblspace TO PUBLIC;
ROLLBACK;
CREATE ROLE regress_tablespace_user1 login;
CREATE ROLE regress_tablespace_user2 login;
GRANT USAGE ON SCHEMA testschema TO regress_tablespace_user2;
Expand Down
6 changes: 6 additions & 0 deletions src/test/regress/sql/tablespace.sql
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,12 @@ ALTER INDEX testschema.part_a_idx SET TABLESPACE pg_default;
-- Fail, not empty
DROP TABLESPACE regress_tblspace;

-- Adequate cache initialization before GRANT
\c -
BEGIN;
GRANT ALL ON TABLESPACE regress_tblspace TO PUBLIC;
ROLLBACK;

CREATE ROLE regress_tablespace_user1 login;
CREATE ROLE regress_tablespace_user2 login;
GRANT USAGE ON SCHEMA testschema TO regress_tablespace_user2;
Expand Down

0 comments on commit e8c78ed

Please sign in to comment.