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 aee976d7ca1dd244ea9ade1c22aaa53169ae6680..b7f35ce7e6e93c9ba7e23b6ec3c69f6afdcc463a 100644 (file)
--- 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 22bfbcf20ea3db6c45f32d9729d951f043b71f44..0b6a2389c05f5af6ada579706ce20213d874961a 100644 (file)
--- 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 1506f413a7b8de642a3ebc3b363ed6669bb9ff5a..ae2ac735c0c128606ea5122e76fd1e8600f554fc 100644 (file)
--- 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;
+}