v8-0002-Fix-misuse-of-simplehash.h-hash-operations-in-pg_.patch

From f28cd200bd5db855164aa06be5c3e960a5ab2f0f Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 25 Mar 2026 13:00:54 -0400
Subject: [PATCH v8 2/2] Fix misuse of simplehash.h hash operations in
 pg_waldump.

Both ArchivedWAL_insert and ArchivedWAL_delete_item can cause
existing hashtable entries to move.  The code didn't account
for this and could leave privateInfo->cur_file pointing at a
dead or incorrect entry, with hilarity ensuing.  Likewise,
read_archive_wal_page calls read_archive_file which could
result in movement of the hashtable entry it is working with
(due to hashtable expansion for insertion of new entries).

I believe these bugs explain some odd buildfarm failures,
although the amount of data we use in pg_waldump's TAP tests
isn't enough to trigger them reliably.
---
 src/bin/pg_waldump/archive_waldump.c | 36 ++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/src/bin/pg_waldump/archive_waldump.c b/src/bin/pg_waldump/archive_waldump.c
index b8e9754b729..4348e192f19 100644
--- a/src/bin/pg_waldump/archive_waldump.c
+++ b/src/bin/pg_waldump/archive_waldump.c
@@ -307,6 +307,7 @@ read_archive_wal_page(XLogDumpPrivate *privateInfo, XLogRecPtr targetPagePtr,
 	XLByteToSeg(targetPagePtr, segno, segsize);
 	XLogFileName(fname, privateInfo->timeline, segno, segsize);
 	entry = get_archive_wal_entry(fname, privateInfo);
+	Assert(!entry->spilled);
 
 	while (nbytes > 0)
 	{
@@ -403,6 +404,14 @@ read_archive_wal_page(XLogDumpPrivate *privateInfo, XLogRecPtr targetPagePtr,
 						 privateInfo->archive_name, fname,
 						 (long long int) (count - nbytes),
 						 (long long int) count);
+
+			/*
+			 * Loading more data may have moved hash table entries, so we must
+			 * re-look-up the one we are reading from.
+			 */
+			entry = ArchivedWAL_lookup(privateInfo->archive_wal_htab, fname);
+			/* ... it had better still be there */
+			Assert(entry != NULL);
 		}
 	}
 
@@ -429,6 +438,7 @@ void
 free_archive_wal_entry(const char *fname, XLogDumpPrivate *privateInfo)
 {
 	ArchivedWALFile *entry;
+	const char *oldfname;
 
 	entry = ArchivedWAL_lookup(privateInfo->archive_wal_htab, fname);
 
@@ -454,7 +464,23 @@ free_archive_wal_entry(const char *fname, XLogDumpPrivate *privateInfo)
 	if (privateInfo->cur_file == entry)
 		privateInfo->cur_file = NULL;
 
+	/*
+	 * ArchivedWAL_delete_item may cause other hash table entries to move.
+	 * Therefore, if cur_file isn't NULL now, we have to be prepared to look
+	 * that entry up again after the deletion.  Fortunately, the entry's fname
+	 * string won't move.
+	 */
+	oldfname = privateInfo->cur_file ? privateInfo->cur_file->fname : NULL;
+
 	ArchivedWAL_delete_item(privateInfo->archive_wal_htab, entry);
+
+	if (oldfname)
+	{
+		privateInfo->cur_file = ArchivedWAL_lookup(privateInfo->archive_wal_htab,
+												   oldfname);
+		/* ... it had better still be there */
+		Assert(privateInfo->cur_file != NULL);
+	}
 }
 
 /*
@@ -700,6 +726,9 @@ astreamer_waldump_content(astreamer *streamer, astreamer_member *member,
 				ArchivedWALFile *entry;
 				bool		found;
 
+				/* Shouldn't see MEMBER_HEADER in the middle of a file */
+				Assert(privateInfo->cur_file == NULL);
+
 				pg_log_debug("reading \"%s\"", member->pathname);
 
 				if (!member_is_wal_file(mystreamer, member, &fname))
@@ -728,6 +757,13 @@ astreamer_waldump_content(astreamer *streamer, astreamer_member *member,
 					}
 				}
 
+				/*
+				 * Note: ArchivedWAL_insert may cause existing hash table
+				 * entries to move.  While cur_file is known to be NULL right
+				 * now, read_archive_wal_page may have a live hash entry
+				 * pointer, which it needs to take care to update after
+				 * read_archive_file completes.
+				 */
 				entry = ArchivedWAL_insert(privateInfo->archive_wal_htab,
 										   fname, &found);
 
-- 
2.43.7