v5-0002-Add-basic_archive-shutdown-callback-to-remove-lef.patch

application/x-patch

Filename: v5-0002-Add-basic_archive-shutdown-callback-to-remove-lef.patch
Type: application/x-patch
Part: 1
Message: Re: thinko in basic_archive.c

Patch

Format: format-patch
Series: patch v5-0002
Subject: Add basic_archive shutdown callback to remove leftover temporary files
File+
contrib/basic_archive/basic_archive.c 30 8
From 22acbf446a4d57fbf1f78985c6c130dcf9871d39 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Wed, 9 Nov 2022 08:44:46 +0000
Subject: [PATCH v5] Add basic_archive shutdown callback to remove leftover
 temporary files

---
 contrib/basic_archive/basic_archive.c | 38 +++++++++++++++++++++------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 1b9f48eeed..19b48818cb 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -42,10 +42,12 @@ PG_MODULE_MAGIC;
 
 static char *archive_directory = NULL;
 static MemoryContext basic_archive_context;
+static char	tempfilepath[MAXPGPATH + 256];
 
 static bool basic_archive_configured(void);
 static bool basic_archive_file(const char *file, const char *path);
 static void basic_archive_file_internal(const char *file, const char *path);
+static void basic_archive_shutdown(void);
 static bool check_archive_directory(char **newval, void **extra, GucSource source);
 static bool compare_files(const char *file1, const char *file2);
 
@@ -85,6 +87,7 @@ _PG_archive_module_init(ArchiveModuleCallbacks *cb)
 
 	cb->check_configured_cb = basic_archive_configured;
 	cb->archive_file_cb = basic_archive_file;
+	cb->shutdown_cb = basic_archive_shutdown;
 }
 
 /*
@@ -215,7 +218,6 @@ static void
 basic_archive_file_internal(const char *file, const char *path)
 {
 	char		destination[MAXPGPATH];
-	char		temp[MAXPGPATH + 256];
 	struct stat st;
 	struct timeval tv;
 	uint64		epoch;			/* milliseconds */
@@ -268,23 +270,23 @@ basic_archive_file_internal(const char *file, const char *path)
 		pg_add_u64_overflow(epoch, (uint64) (tv.tv_usec / 1000), &epoch))
 		elog(ERROR, "could not generate temporary file name for archiving");
 
-	snprintf(temp, sizeof(temp), "%s/%s.%s.%d." UINT64_FORMAT,
+	snprintf(tempfilepath, sizeof(tempfilepath), "%s/%s.%s.%d." UINT64_FORMAT,
 			 archive_directory, "archtemp", file, MyProcPid, epoch);
 
 	/*
 	 * Copy the file to its temporary destination.  Note that this will fail
 	 * if temp already exists.
 	 */
-	if (copy_file(unconstify(char *, path), temp, LOG) != 0)
+	if (copy_file(unconstify(char *, path), tempfilepath, LOG) != 0)
 	{
 		/* Remove the leftover temporary file. */
 		if (errno != EEXIST)
-			unlink(temp);
+			unlink(tempfilepath);
 
 		ereport(ERROR,
 				(errcode_for_file_access(),
 				 errmsg("could not copy file \"%s\" to temporary file \"%s\": %m",
-						path, temp)));
+						path, tempfilepath)));
 	}
 
 	/*
@@ -292,17 +294,24 @@ basic_archive_file_internal(const char *file, const char *path)
 	 * Note that this will overwrite any existing file, but this is only
 	 * possible if someone else created the file since the stat() above.
 	 */
-	if (durable_rename(temp, destination, LOG) != 0)
+	if (durable_rename(tempfilepath, destination, LOG) != 0)
 	{
 		/* Remove the leftover temporary file. */
-		unlink(temp);
+		unlink(tempfilepath);
 
 		ereport(ERROR,
 				(errcode_for_file_access(),
 				 errmsg("could not rename temporary file \"%s\" to \"%s\": %m",
-						temp, destination)));
+						tempfilepath, destination)));
 	}
 
+	/*
+	 * Reset tempfilepath after renaming the temporary file to the final file
+	 * so that the shutdown callback, if called after this point, will not
+	 * attempt to remove it and fail.
+	 */
+	tempfilepath[0] = '\0';
+
 	ereport(DEBUG1,
 			(errmsg("archived \"%s\" via basic_archive", file)));
 }
@@ -387,3 +396,16 @@ compare_files(const char *file1, const char *file2)
 
 	return ret;
 }
+
+static void
+basic_archive_shutdown(void)
+{
+	if (tempfilepath[0] == '\0')
+		return;
+
+	/* Remove the leftover temporary file. */
+	if (unlink(tempfilepath) < 0)
+		ereport(WARNING,
+				(errcode_for_file_access(),
+				 errmsg("could not unlink file \"%s\": %m", tempfilepath)));
+}
-- 
2.34.1