public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tatsuo Ishii <[email protected]>
To: [email protected]
Subject: Lock release leak in pool_search_relcache
Date: Tue, 17 Mar 2026 18:26:15 +0900 (JST)
Message-ID: <[email protected]> (raw)
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
Attachments:
[text/x-patch] fix_relcache.patch (3.0K, 2-fix_relcache.patch)
download | inline diff:
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);
reply
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: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Lock release leak in pool_search_relcache
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox