From: Noah Misch <noah@leadboat.com>
To: Aya Iwata (Fujitsu) <iwata.aya@fujitsu.com>
Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: Re: 'WARNING: you don't own a lock of type ExclusiveLock' is printed during the GRANT command on PostgreSQL16.5
Date: Wed, 20 Nov 2024 17:11:00 -0800
Message-ID: <20241121011100.c3.nmisch@google.com> (raw)
In-Reply-To: <20241120062742.b0.nmisch@google.com>
References: <OS7PR01MB11964507B5548245A7EE54E70EA212@OS7PR01MB11964.jpnprd01.prod.outlook.com>
<20241120062742.b0.nmisch@google.com>
On Tue, Nov 19, 2024 at 10:27:42PM -0800, Noah Misch wrote:
> On Wed, Nov 20, 2024 at 05:11:07AM +0000, Aya Iwata (Fujitsu) wrote:
> > During our tests on November release,
> > we discovered a "WARNING: you don't own a lock of type ExclusiveLock" that occurred at a strange time in GRANT command.
> > This is only possible when psql executes each command individually.
> > $ psql -d postgres -c "CREATE USER nmluser1;"
> > CREATE ROLE
> > $ psql -d postgres -c "CREATE TABLESPACE test_tblspace LOCATION '/xxx /xxx/pgsql16.5/tbs';"
> > CREATE TABLESPACE
> > $ psql -d postgres -c "CREATE DATABASE enc_testdb;"
> > CREATE DATABASE
> > $ psql -d postgres -c "GRANT ALL PRIVILEGES ON DATABASE postgres TO nmluser1;"
> > GRANT
> > $ psql -d postgres -c "GRANT ALL PRIVILEGES ON DATABASE enc_testdb TO nmluser1;"
> > GRANT
> > $ psql -d postgres -c "GRANT CREATE ON TABLESPACE test_tblspace TO nmluser1;"
> > WARNING: you don't own a lock of type ExclusiveLock
> > GRANT
Did you ever see the WARNING with a DATABASE command, or just with a
TABLESPACE command?
> Thanks for the report. This seems reasonably likely to come from a defect in
> commit 51ff46d. I will look into it.
Yes, that commit had us reading cc_relisshared before that field got
initialized. I can reproduce the WARNING in v16+, with GRANT TABLESPACE.
Before a cache's initialization cc_relisshared==false, so
SearchSysCacheLocked1() used the wrong locktag. The attached patch is about
right. At a minimum, I still need to work on the details in its log message.
Attachments:inplace250-catcache-init-SearchSysCacheLocked1-v1.patch (text/plain, 3.7K) download
Author: Noah Misch <noah@leadboat.com>
Commit: Noah Misch <noah@leadboat.com>
Avoid "you don't own a lock of type ExclusiveLock" in GRANT TABLESPACE.
FIXME look for other user-visible implications:
- other commands that can reach this warning
- commands that actually need the protection of the LOCKTAG_TUPLE and
don't get it as a result of setting the wrong dboid in the locktag.
These would be commands involving pg_database.
This happened because SearchSysCacheLocked1() read cc_relisshared before
cache initialization, so the locktag for a shared catalog incorrectly
contained dboid==MyDatabaseId. Only shared catalogs were affected.
Back-patch to v13 (all supported versions). This has no known impact
before v16, where ExecGrant_common() first appeared. Earlier branches
have a separate ExecGrant_Tablespace() that doesn't use LOCKTAG_TUPLE,
and ExecGrant_Database() seems to get a correctly-initialized cache
FIXMEconfirm. Even so it's not very future-proof to assume v15 won't
get impact in the future.
Reported by Aya Iwata. Reviewed by FIXME.
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.cindex f41b1c2..f7f4f56 100644--- a/src/backend/utils/cache/syscache.c+++ b/src/backend/utils/cache/syscache.c@@ -287,11 +287,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
@@ -343,8 +341,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/expected/tablespace.out b/src/test/regress/expected/tablespace.outindex dd535d4..a90e39e 100644--- a/src/test/regress/expected/tablespace.out+++ b/src/test/regress/expected/tablespace.out@@ -927,6 +927,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;
diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sqlindex c8b8378..dfe3db0 100644--- a/src/test/regress/sql/tablespace.sql+++ b/src/test/regress/sql/tablespace.sql@@ -396,6 +396,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;
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: pgsql-bugs@postgresql.org
Cc: noah@leadboat.com, iwata.aya@fujitsu.com, pgsql-bugs@lists.postgresql.org
Subject: Re: 'WARNING: you don't own a lock of type ExclusiveLock' is printed during the GRANT command on PostgreSQL16.5
In-Reply-To: <20241121011100.c3.nmisch@google.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox