Avoid "you don't own a lock of type ExclusiveLock" in GRANT TABLESPACE.

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- 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- of a SearchSysCacheLocked1() call.

Reported by Aya Iwata.

Discussion: https://postgr.es/m/OS7PR01MB11964507B5548245A7EE54E70EA212@OS7PR01MB11964.jpnprd01.prod.outlook.com

diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c
index 8ad87914d4d7831aa15de027ec22b463d5b0d79c..8856ae1508f0a1917cd0f1e87f9e0b6858c27319 100644 (file)
--- a/src/backend/utils/cache/syscache.c
+++ b/src/backend/utils/cache/syscache.c
@@ -1185,11 +1185,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
@@ -1241,8 +1239,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);

diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source
index 1a181016d716ac4cacccaff941d46e39be39a53a..409caa42be36c8156ff317c27c547dbf3bd1667c 100644 (file)
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/input/tablespace.source
@@ -255,6 +255,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;

diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source
index 94c5f023c68d4d3c320bde2a73320b871a2cfdc4..7e89657ab9daa9ac3d1b2366b5bd57c658c3bc3e 100644 (file)
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -720,6 +720,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;