From: Tatsuo Ishii Date: Fri, 21 Jun 2024 06:37:25 +0000 (+0900) Subject: Fix segfault to not use MAIN_NODE macro. X-Git-Tag: V4_6_0_BETA1~81 X-Git-Url: http://git.postgresql.org/gitweb/static/gitweb.js?a=commitdiff_plain;h=98fff9832f602ea5571427982f1050348f9f0219;p=pgpool2.git Fix segfault to not use MAIN_NODE macro. Some functions (close_idle_connection(), new_connection() and pool_create_cp()) used MAIN* and VALID_BACKEND where they are not appropriate. MAIN* and VALID_BACKEND are only useful against current connections to backend, not for pooled connections since in pooled connections which backend is the main node or up and running is necessarily same as the current connections to backend. The misuses of those macros sometimes leads to segfault. This patch introduces new in_use_backend_id() which returns the fist node id in use. This commit replaces some of MAIN* with the return value from in_use_backend_id(). Also inappropriate calls to VALID_BACKEND are replaced with CONNECTION_SLOT macro. Problem reported by Emond Papegaaij Discussion: https://www.pgpool.net/pipermail/pgpool-general/2024-June/009176.html [pgpool-general: 9114] Re: Another segmentation fault Backpatch-through: V4.1 --- diff --git a/src/include/protocol/pool_connection_pool.h b/src/include/protocol/pool_connection_pool.h index aee976d7c..b7f35ce7e 100644 --- a/src/include/protocol/pool_connection_pool.h +++ b/src/include/protocol/pool_connection_pool.h @@ -3,7 +3,7 @@ * pgpool: a language independent connection pool server for PostgreSQL * written by Tatsuo Ishii * - * Copyright (c) 2003-2020 PgPool Global Development Group + * Copyright (c) 2003-2024 PgPool Global Development Group * * Permission to use, copy, modify, and distribute this software and * its documentation for any purpose and without fee is hereby @@ -38,4 +38,6 @@ extern int connect_unix_domain_socket_by_port(int port, char *socket_dir, bool r extern int pool_pool_index(void); extern void close_all_backend_connections(void); extern void update_pooled_connection_count(void); +extern int in_use_backend_id(POOL_CONNECTION_POOL *pool); + #endif /* pool_connection_pool_h */ diff --git a/src/protocol/child.c b/src/protocol/child.c index 22bfbcf20..0b6a2389c 100644 --- a/src/protocol/child.c +++ b/src/protocol/child.c @@ -1193,6 +1193,7 @@ static RETSIGTYPE close_idle_connection(int sig) POOL_CONNECTION_POOL *p = pool_connection_pool; ConnectionInfo *info; int save_errno = errno; + int main_node_id; /* * DROP DATABASE is ongoing. @@ -1200,44 +1201,35 @@ static RETSIGTYPE close_idle_connection(int sig) if (ignore_sigusr1) return; -#ifdef NOT_USED - ereport(DEBUG1, - (errmsg("close connection request received"))); -#endif - for (j = 0; j < pool_config->max_pool; j++, p++) { - if (!MAIN_CONNECTION(p)) + main_node_id = in_use_backend_id(p); + if (main_node_id < 0) + continue; + + if (!CONNECTION_SLOT(p, main_node_id)) continue; - if (!MAIN_CONNECTION(p)->sp) + if (!CONNECTION_SLOT(p, main_node_id)->sp) continue; - if (MAIN_CONNECTION(p)->sp->user == NULL) + if (CONNECTION_SLOT(p, main_node_id)->sp->user == NULL) continue; - if (MAIN_CONNECTION(p)->closetime > 0) /* idle connection? */ + if (CONNECTION_SLOT(p, main_node_id)->closetime > 0) /* idle connection? */ { -#ifdef NOT_USED - ereport(DEBUG1, - (errmsg("closing idle connection"), - errdetail("user: %s database: %s", MAIN_CONNECTION(p)->sp->user, MAIN_CONNECTION(p)->sp->database))); -#endif + bool freed = false; pool_send_frontend_exits(p); for (i = 0; i < NUM_BACKENDS; i++) { - if (!VALID_BACKEND(i)) + if (!CONNECTION_SLOT(p, i)) continue; - if (i == 0) + if (!freed) { - /* - * only first backend allocated the memory for the start - * up packet - */ pool_free_startup_packet(CONNECTION_SLOT(p, i)->sp); CONNECTION_SLOT(p, i)->sp = NULL; - + freed = true; } pool_close(CONNECTION(p, i)); } diff --git a/src/protocol/pool_connection_pool.c b/src/protocol/pool_connection_pool.c index 1506f413a..ae2ac735c 100644 --- a/src/protocol/pool_connection_pool.c +++ b/src/protocol/pool_connection_pool.c @@ -68,6 +68,8 @@ static POOL_CONNECTION_POOL * new_connection(POOL_CONNECTION_POOL * p); static int check_socket_status(int fd); static bool connect_with_timeout(int fd, struct addrinfo *walk, char *host, int port, bool retry); +#define TMINTMAX 0x7fffffff + /* * initialize connection pools. this should be called once at the startup. */ @@ -255,6 +257,7 @@ pool_create_cp(void) POOL_CONNECTION_POOL *oldestp; POOL_CONNECTION_POOL *ret; ConnectionInfo *info; + int main_node_id; POOL_CONNECTION_POOL *p = pool_connection_pool; @@ -267,7 +270,7 @@ pool_create_cp(void) for (i = 0; i < pool_config->max_pool; i++) { - if (MAIN_CONNECTION(p) == NULL) + if (in_use_backend_id(p) < 0) /* is this connection pool out of use? */ { ret = new_connection(p); if (ret) @@ -285,21 +288,25 @@ pool_create_cp(void) * discard it. */ oldestp = p = pool_connection_pool; - closetime = MAIN_CONNECTION(p)->closetime; + closetime = TMINTMAX; pool_index = 0; for (i = 0; i < pool_config->max_pool; i++) { + main_node_id = in_use_backend_id(p); + if (main_node_id < 0) + elog(ERROR, "no in use backend found"); /* this should not happen */ + ereport(DEBUG1, (errmsg("creating connection pool"), errdetail("user: %s database: %s closetime: %ld", - MAIN_CONNECTION(p)->sp->user, - MAIN_CONNECTION(p)->sp->database, - MAIN_CONNECTION(p)->closetime))); + CONNECTION_SLOT(p, main_node_id)->sp->user, + CONNECTION_SLOT(p, main_node_id)->sp->database, + CONNECTION_SLOT(p, main_node_id)->closetime))); - if (MAIN_CONNECTION(p)->closetime < closetime) + if (CONNECTION_SLOT(p, main_node_id)->closetime < closetime) { - closetime = MAIN_CONNECTION(p)->closetime; + closetime = CONNECTION_SLOT(p, main_node_id)->closetime; oldestp = p; pool_index = i; } @@ -307,18 +314,21 @@ pool_create_cp(void) } p = oldestp; + main_node_id = in_use_backend_id(p); + if (main_node_id < 0) + elog(ERROR, "no in use backend found"); /* this should not happen */ pool_send_frontend_exits(p); ereport(DEBUG1, (errmsg("creating connection pool"), errdetail("discarding old %zd th connection. user: %s database: %s", oldestp - pool_connection_pool, - MAIN_CONNECTION(p)->sp->user, - MAIN_CONNECTION(p)->sp->database))); + CONNECTION_SLOT(p, main_node_id)->sp->user, + CONNECTION_SLOT(p, main_node_id)->sp->database))); for (i = 0; i < NUM_BACKENDS; i++) { - if (!VALID_BACKEND(i)) + if (CONNECTION_SLOT(p, i) == NULL) continue; if (!freed) @@ -399,8 +409,6 @@ pool_backend_timer_handler(int sig) void pool_backend_timer(void) { -#define TMINTMAX 0x7fffffff - POOL_CONNECTION_POOL *p = pool_connection_pool; int i, j; @@ -1088,3 +1096,21 @@ void update_pooled_connection_count(void) } pool_get_my_process_info()->pooled_connections = count; } + +/* + * Return the first node id in use. + * If no node is in use, return -1. + */ +int +in_use_backend_id(POOL_CONNECTION_POOL *pool) +{ + int i; + + for (i = 0; i < NUM_BACKENDS; i++) + { + if (pool->slots[i]) + return i; + } + + return -1; +}