Date: Fri, 03 Jul 2026 20:24:23 +0900 (JST)
Message-Id: <20260703.202423.640416716760511415.ishii@postgresql.org>
To: pgpool-hackers@lists.postgresql.org
Subject: Re: Adding volatile qualifer
From: Tatsuo Ishii <ishii@postgresql.org>
In-Reply-To: <20260701.101148.451717746326656928.ishii@postgresql.org>
References: <20260701.101148.451717746326656928.ishii@postgresql.org>
Mime-Version: 1.0
Content-Type: Multipart/Mixed;
 boundary="--Next_Part(Fri_Jul__3_20_24_23_2026_338)--"
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20260703.202423.640416716760511415.ishii%40postgresql.org>
Precedence: bulk

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

> While looking into the Pgpool-II main source code, I noticed that
> local pointer array "my_backend_status" is not volatile qualified,
> while it should have been.
> 
> BACKEND_STATUS *my_backend_status[MAX_NUM_BACKENDS];	/* Backend status buffer */
> 
> This array members are initialized:
> 
> 	for (i = 0; i < MAX_NUM_BACKENDS; i++)
> 	{
> 		my_backend_status[i] = &(BACKEND_INFO(i).backend_status);
> 	}
> 
> Without volatile, *(my_backend_status[i]) could read state value
> because of compiler optimization. Since my_backend_status[i] is
> referred to in popular VALID_BACKEND macro, we should fix it in all
> supported branches.

volatile qualifier should have been attached to the right hand side
too. Attached v2 patch does it.

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


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

diff --git a/src/include/pool.h b/src/include/pool.h
index 549aed30f..ef0876c84 100644
--- a/src/include/pool.h
+++ b/src/include/pool.h
@@ -345,7 +345,7 @@ extern int	pool_get_major_version(void);
 extern bool pool_is_node_to_be_sent_in_current_query(int node_id);
 extern int	pool_virtual_main_db_node_id(void);
 
-extern BACKEND_STATUS *my_backend_status[];
+extern volatile BACKEND_STATUS *my_backend_status[];
 extern int	my_main_node_id;
 
 #define VALID_BACKEND(backend_id) \
diff --git a/src/main/pgpool_main.c b/src/main/pgpool_main.c
index 19d89cb79..14d238003 100644
--- a/src/main/pgpool_main.c
+++ b/src/main/pgpool_main.c
@@ -277,7 +277,9 @@ static pid_t pgpool_logger_pid = 0; /* pid for pgpool_logger process */
 static pid_t wd_lifecheck_pid = 0;	/* pid for child process handling watchdog
 									 * lifecheck */
 
-BACKEND_STATUS *my_backend_status[MAX_NUM_BACKENDS];	/* Backend status buffer */
+/* Backend status buffer */
+volatile BACKEND_STATUS *my_backend_status[MAX_NUM_BACKENDS];
+
 int			my_main_node_id;	/* Main node id buffer */
 
 /*
@@ -3220,7 +3222,8 @@ initialize_shared_mem_objects(bool clear_memcache_oidmaps)
 
 	for (i = 0; i < MAX_NUM_BACKENDS; i++)
 	{
-		my_backend_status[i] = &(BACKEND_INFO(i).backend_status);
+		my_backend_status[i] =
+			(volatile BACKEND_STATUS *) &(BACKEND_INFO(i).backend_status);
 	}
 
 	/* initialize Req_info */
@@ -3783,7 +3786,8 @@ sync_backend_from_watchdog(void)
 			{
 				BACKEND_INFO(i).backend_status = CON_DOWN;
 				pool_set_backend_status_changed_time(i);
-				my_backend_status[i] = &(BACKEND_INFO(i).backend_status);
+				my_backend_status[i] =
+					(volatile BACKEND_STATUS *) &(BACKEND_INFO(i).backend_status);
 				reload_master_node_id = true;
 				node_status_was_changed_to_down = true;
 				ereport(LOG,
@@ -3802,7 +3806,8 @@ sync_backend_from_watchdog(void)
 
 				BACKEND_INFO(i).backend_status = CON_CONNECT_WAIT;
 				pool_set_backend_status_changed_time(i);
-				my_backend_status[i] = &(BACKEND_INFO(i).backend_status);
+				my_backend_status[i] =
+					(volatile BACKEND_STATUS *) &(BACKEND_INFO(i).backend_status);
 				reload_master_node_id = true;
 
 				ereport(LOG,

----Next_Part(Fri_Jul__3_20_24_23_2026_338)----