Date: Tue, 17 Mar 2026 18:26:15 +0900 (JST)
Message-Id: <20260317.182615.1046517700474336117.ishii@postgresql.org>
To: pgpool-hackers@lists.postgresql.org
Subject: Lock release leak in pool_search_relcache
From: Tatsuo Ishii <ishii@postgresql.org>
Mime-Version: 1.0
Content-Type: Multipart/Mixed;
 boundary="--Next_Part(Tue_Mar_17_18_26_15_2026_580)--"
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20260317.182615.1046517700474336117.ishii%40postgresql.org>
Precedence: bulk

----Next_Part(Tue_Mar_17_18_26_15_2026_580)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

While looking into src/utils/pool_relcache.c for other reasons, I
noticed pool_search_relcache does not release lock in a certain code
path.  When the local relation cache does not hit, it searches the
shared relation cache if enable_shared_relcache is on. If the lock for
query cache has been already held (thus the variable "locked" is set
to true), it acquires a shared lock, then releases it and acquires an
exclusive lock to call pool_catalog_commit_cache. Since in this code
path variable "locked" is set to true, subsequent code:

	if (pool_config->enable_shared_relcache && !locked)
	{
		pool_shmem_unlock();
		POOL_SETMASK(&oldmask);
	}

does not reach inside the if statement -- lock is not released. I
think to fix this, the following code requires "locked = false;"

			pool_shmem_unlock();
			pool_shmem_lock(POOL_MEMQ_EXCLUSIVE_LOCK);
			pool_catalog_commit_cache(backend, query, query_cache_data, query_cache_len);

Attached patch does this. Plus following changes are made in the patch:

- the variable name "locked" is confusing. To clarify the role of the
  variable, its name is changed to "locked_by_others".

- Fix per_node_statement_log is called even though actual query is
  not sent to backend.

Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

----Next_Part(Tue_Mar_17_18_26_15_2026_580)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="fix_relcache.patch"

diff --git a/src/utils/pool_relcache.c b/src/utils/pool_relcache.c
index d4ebab25e..0eef6b182 100644
--- a/src/utils/pool_relcache.c
+++ b/src/utils/pool_relcache.c
@@ -5,7 +5,7 @@
  * pgpool: a language independent connection pool server for PostgreSQL
  * written by Tatsuo Ishii
  *
- * Copyright (c) 2003-2025	PgPool Global Development Group
+ * Copyright (c) 2003-2026	PgPool Global Development Group
  *
  * Permission to use, copy, modify, and distribute this software and
  * its documentation for any purpose and without fee is hereby
@@ -118,7 +118,10 @@ pool_search_relcache(POOL_RELCACHE *relcache, POOL_CONNECTION_POOL *backend, cha
 	void	   *result;
 	ErrorContextCallback callback;
 	pool_sigset_t oldmask;
-	bool		locked;
+	/*
+	 * True if locked has been already acquired by others.
+	 */
+	bool		locked_by_others;
 	int			query_cache_not_found = 1;
 	char	   *query_cache_data = NULL;
 	size_t		query_cache_len;
@@ -206,11 +209,11 @@ pool_search_relcache(POOL_RELCACHE *relcache, POOL_CONNECTION_POOL *backend, cha
 		}
 	}
 
-	/* Not in cache. Check the system catalog */
+	/*
+	 * Not in cache. Form a query to check the system catalog.
+	 */
 	snprintf(query, sizeof(query), relcache->sql, table);
 
-	per_node_statement_log(backend, node_id, query);
-
 	/*
 	 * Register a error context callback to throw proper context message
 	 */
@@ -219,7 +222,7 @@ pool_search_relcache(POOL_RELCACHE *relcache, POOL_CONNECTION_POOL *backend, cha
 	callback.previous = error_context_stack;
 	error_context_stack = &callback;
 
-	locked = pool_is_shmem_lock();
+	locked_by_others = pool_is_shmem_lock();
 
 	/*
 	 * if enable_shared_relcache is true, search query cache.
@@ -227,7 +230,7 @@ pool_search_relcache(POOL_RELCACHE *relcache, POOL_CONNECTION_POOL *backend, cha
 	if (pool_config->enable_shared_relcache)
 	{
 		/* if shmem is not locked by this process, get the lock */
-		if (!locked)
+		if (!locked_by_others)
 		{
 			POOL_SETMASK2(&BlockSig, &oldmask);
 			pool_shmem_lock(POOL_MEMQ_SHARED_LOCK);
@@ -253,6 +256,8 @@ pool_search_relcache(POOL_RELCACHE *relcache, POOL_CONNECTION_POOL *backend, cha
 				 errdetail("query:%s", query)));
 
 		do_query(CONNECTION(backend, node_id), query, &res, MAJOR(backend));
+		per_node_statement_log(backend, node_id, query);
+
 		/* Register cache */
 		result = (*relcache->register_func) (res);
 		/* save local catalog cache in query cache */
@@ -273,6 +278,7 @@ pool_search_relcache(POOL_RELCACHE *relcache, POOL_CONNECTION_POOL *backend, cha
 			 */
 			pool_shmem_unlock();
 			pool_shmem_lock(POOL_MEMQ_EXCLUSIVE_LOCK);
+			locked_by_others = false;
 			pool_catalog_commit_cache(backend, query, query_cache_data, query_cache_len);
 		}
 	}
@@ -287,7 +293,7 @@ pool_search_relcache(POOL_RELCACHE *relcache, POOL_CONNECTION_POOL *backend, cha
 		result = (*relcache->register_func) (res);
 	}
 	/* if shmem is locked by this function, unlock it */
-	if (pool_config->enable_shared_relcache && !locked)
+	if (pool_config->enable_shared_relcache && !locked_by_others)
 	{
 		pool_shmem_unlock();
 		POOL_SETMASK(&oldmask);

----Next_Part(Tue_Mar_17_18_26_15_2026_580)----