public inbox for [email protected]
help / color / mirror / Atom feedLock release leak in pool_search_relcache
2+ messages / 1 participants
[nested] [flat]
* Lock release leak in pool_search_relcache
@ 2026-03-17 09:26 Tatsuo Ishii <[email protected]>
2026-03-19 06:37 ` Re: Lock release leak in pool_search_relcache Tatsuo Ishii <[email protected]>
0 siblings, 1 reply; 2+ messages in thread
From: Tatsuo Ishii @ 2026-03-17 09:26 UTC (permalink / raw)
To: [email protected]
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);
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: Lock release leak in pool_search_relcache
2026-03-17 09:26 Lock release leak in pool_search_relcache Tatsuo Ishii <[email protected]>
@ 2026-03-19 06:37 ` Tatsuo Ishii <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Tatsuo Ishii @ 2026-03-19 06:37 UTC (permalink / raw)
To: [email protected]
> 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.
Pushed down to v4.4 stable.
Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-03-19 06:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-03-17 09:26 Lock release leak in pool_search_relcache Tatsuo Ishii <[email protected]>
2026-03-19 06:37 ` Tatsuo Ishii <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox