From 1c998da8805cb172a4d5e3fe322a6984e1ba1e39 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Sat, 18 May 2024 13:41:09 +1200 Subject: [PATCH v2] Fix pg_control corruption in EXEC_BACKEND startup. When backend processes were launched in EXEC_BACKEND builds, they would run LocalProcessControlFile() to read in pg_control and extract several important settings. This happens too early to acquire ControlFileLock, and the postmaster is also not allowed to acquire ControlFileLock, so it can't safely take a copy to give to the child. Instead, pass down the "proto-controlfile" that was read by the postmaster in LocalProcessControlFile(). Introduce functions ExportProtoControlFile() and ImportProtoControlFile() to allow that. Subprocesses will extract information from that, and then later attach to the current control file in shared memory. Reported-by: Melanie Plageman per Windows CI failure Discussion: https://postgr.es/m/CAAKRu_YNGwEYrorQYza_W8tU%2B%3DtoXRHG8HpyHC-KDbZqA_ZVSA%40mail.gmail.com --- src/backend/access/transam/xlog.c | 46 +++++++++++++++++++++++-- src/backend/postmaster/launch_backend.c | 21 +++++++---- src/include/access/xlog.h | 5 +++ 3 files changed, 64 insertions(+), 8 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 13ec6225b85..e52517eb9c1 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -575,6 +575,10 @@ static WALInsertLockPadded *WALInsertLocks = NULL; */ static ControlFileData *ControlFile = NULL; +#ifdef EXEC_BACKEND +static ControlFileData *ProtoControlFile = NULL; +#endif + /* * Calculate the amount of space left on the page after 'endptr'. Beware * multiple evaluation! @@ -692,6 +696,7 @@ static bool PerformRecoveryXLogAction(void); static void InitControlFile(uint64 sysidentifier, uint32 data_checksum_version); static void WriteControlFile(void); static void ReadControlFile(void); +static void ScanControlFile(void); static void UpdateControlFile(void); static char *str_time(pg_time_t tnow, char *buf, size_t bufsize); @@ -4385,9 +4390,7 @@ WriteControlFile(void) static void ReadControlFile(void) { - pg_crc32c crc; int fd; - char wal_segsz_str[20]; int r; /* @@ -4420,6 +4423,15 @@ ReadControlFile(void) close(fd); + ScanControlFile(); +} + +static void +ScanControlFile(void) +{ + static char wal_segsz_str[20]; + pg_crc32c crc; + /* * Check for expected pg_control format version. If this is wrong, the * CRC check will likely fail because we'll be checking the wrong number @@ -4941,8 +4953,33 @@ LocalProcessControlFile(bool reset) Assert(reset || ControlFile == NULL); ControlFile = palloc_object(ControlFileData); ReadControlFile(); + +#ifdef EXEC_BACKEND + /* We need to be able to give this to subprocesses. */ + ProtoControlFile = ControlFile; +#endif } +#ifdef EXEC_BACKEND +void +ExportProtoControlFile(ControlFileData *copy) +{ + *copy = *ProtoControlFile; +} + +/* + * Like LocalProcessControlFile(), but used early in EXEC_BACKEND children's + * startup. This receives the same file that the postmaster first read. + */ +void +ImportProtoControlFile(const ControlFileData *copy) +{ + ControlFile = palloc(sizeof(ControlFileData)); + *ControlFile = *copy; + ScanControlFile(); +} +#endif + /* * Get the wal_level from the control file. For a standby, this value should be * considered as its active wal_level, because it may be different from what @@ -5061,7 +5098,12 @@ XLOGShmemInit(void) if (localControlFile) { memcpy(ControlFile, localControlFile, sizeof(ControlFileData)); +#ifdef EXEC_BACKEND + /* We still hold a reference to give to subprocesses. */ + Assert(ProtoControlFile == localControlFile); +#else pfree(localControlFile); +#endif } /* diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index 45690b11c99..e08a405f949 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -33,6 +33,9 @@ #include +#include "access/xlog.h" +#include "catalog/pg_control.h" +#include "common/file_utils.h" #include "libpq/libpq-be.h" #include "miscadmin.h" #include "postmaster/autovacuum.h" @@ -133,6 +136,14 @@ typedef struct int MyPMChildSlot; + /* + * A copy of the ControlFileData from early in Postmaster startup. We + * need to access its contents it at a phase of initialization before we + * are allowed to acquire LWLocks, so we can't just use shared memory or + * read the file from disk. + */ + ControlFileData proto_controlfile; + /* * These are only used by backend processes, but are here because passing * a socket needs some special handling on Windows. 'client_sock' is an @@ -659,12 +670,6 @@ SubPostmasterMain(int argc, char *argv[]) */ checkDataDir(); - /* - * (re-)read control file, as it contains config. The postmaster will - * already have read this, but this process doesn't know about that. - */ - LocalProcessControlFile(false); - /* * Reload any libraries that were preloaded by the postmaster. Since we * exec'd this process, those libraries didn't come along with us; but we @@ -752,6 +757,8 @@ save_backend_variables(BackendParameters *param, param->MaxBackends = MaxBackends; param->num_pmchild_slots = num_pmchild_slots; + ExportProtoControlFile(¶m->proto_controlfile); + #ifdef WIN32 param->PostmasterHandle = PostmasterHandle; if (!write_duplicated_handle(¶m->initial_signal_pipe, @@ -1026,6 +1033,8 @@ restore_backend_variables(BackendParameters *param) strlcpy(pkglib_path, param->pkglib_path, MAXPGPATH); + ImportProtoControlFile(¶m->proto_controlfile); + /* * We need to restore fd.c's counts of externally-opened FDs; to avoid * confusion, be sure to do this after restoring max_safe_fds. (Note: diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index fdfb572467b..f1f3ad4e96e 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -208,6 +208,7 @@ typedef enum WALAvailability struct XLogRecData; struct XLogReaderState; +struct ControlFileData; extern XLogRecPtr XLogInsertRecord(struct XLogRecData *rdata, XLogRecPtr fpw_lsn, @@ -250,6 +251,10 @@ extern void XLOGShmemInit(void); extern void BootStrapXLOG(uint32 data_checksum_version); extern void InitializeWalConsistencyChecking(void); extern void LocalProcessControlFile(bool reset); +#ifdef EXEC_BACKEND +extern void ExportProtoControlFile(struct ControlFileData *copy); +extern void ImportProtoControlFile(const struct ControlFileData *copy); +#endif extern WalLevel GetActiveWalLevelOnStandby(void); extern void StartupXLOG(void); extern void ShutdownXLOG(int code, Datum arg); -- 2.47.3