Fix file descriptor leakages in pg_waldump.

TarWALDumpCloseSegment was of the opinion that it didn't need to
do anything.  It was mistaken: it has to close the open file if
any, because nothing else will, leading to a descriptor leak.

In addition, we failed to ensure that any file being read by the
XLogReader machinery gets closed before the atexit callback tries to
cleanup the temporary directory holding spilled WAL files.  While the
file would have been closed already in case of a success exit, this
doesn't happen in case of pg_fatal() exits.  The least messy way
to fix that is to move the atexit function into pg_waldump.c,
where it has easier access to the XLogReaderState pointer and to
WALDumpCloseSegment.

These FD leakages are pretty insignificant on Unix-ish platforms,
but they're a bug on Windows, because they prevent successful cleanup
of the temporary directory for extracted WAL files.  (Windows can't
delete a directory that holds a deleted-but-still-open file.)
This is visible in occasional buildfarm failures.

This code's all new as of commit b15c15139, so no need for back-patch.

Discussion: https://postgr.es/m/374225.1774459521@sss.pgh.pa.us
This commit is contained in:
Tom Lane 2026-03-25 18:28:42 -04:00
parent 497c1170cb
commit 03b1e30e7a
2 changed files with 44 additions and 26 deletions

View file

@ -91,7 +91,6 @@ static ArchivedWALFile *get_archive_wal_entry(const char *fname,
XLogDumpPrivate *privateInfo);
static bool read_archive_file(XLogDumpPrivate *privateInfo);
static void setup_tmpwal_dir(const char *waldir);
static void cleanup_tmpwal_dir_atexit(void);
static FILE *prepare_tmp_write(const char *fname, XLogDumpPrivate *privateInfo);
static void perform_tmp_write(const char *fname, StringInfo buf, FILE *file);
@ -607,23 +606,10 @@ setup_tmpwal_dir(const char *waldir)
pg_log_debug("created directory \"%s\"", TmpWalSegDir);
}
/*
* Remove temporary directory at exit, if any.
*/
static void
cleanup_tmpwal_dir_atexit(void)
{
Assert(TmpWalSegDir != NULL);
rmtree(TmpWalSegDir, true);
TmpWalSegDir = NULL;
}
/*
* Open a file in the temporary spill directory for writing an out-of-order
* WAL segment, creating the directory and registering the cleanup callback
* if not already done. Returns the open file handle.
* WAL segment, creating the directory if not already done.
* Returns the open file handle.
*/
static FILE *
prepare_tmp_write(const char *fname, XLogDumpPrivate *privateInfo)
@ -631,15 +617,9 @@ prepare_tmp_write(const char *fname, XLogDumpPrivate *privateInfo)
char fpath[MAXPGPATH];
FILE *file;
/*
* Setup temporary directory to store WAL segments and set up an exit
* callback to remove it upon completion if not already.
*/
/* Setup temporary directory to store WAL segments, if we didn't already */
if (unlikely(TmpWalSegDir == NULL))
{
setup_tmpwal_dir(privateInfo->archive_dir);
atexit(cleanup_tmpwal_dir_atexit);
}
snprintf(fpath, MAXPGPATH, "%s/%s", TmpWalSegDir, fname);

View file

@ -42,6 +42,8 @@ static const char *progname;
static volatile sig_atomic_t time_to_stop = false;
static XLogReaderState *xlogreader_state_cleanup = NULL;
static const RelFileLocator emptyRelFileLocator = {0, 0, 0};
typedef struct XLogDumpConfig
@ -454,13 +456,14 @@ TarWALDumpOpenSegment(XLogReaderState *state, XLogSegNo nextSegNo,
/*
* pg_waldump's XLogReaderRoutine->segment_close callback to support dumping
* WAL files from tar archives. Segment tracking is handled by
* TarWALDumpReadPage, so no action is needed here.
* WAL files from tar archives. Same as wal_segment_close.
*/
static void
TarWALDumpCloseSegment(XLogReaderState *state)
{
/* No action needed */
close(state->seg.ws_file);
/* need to check errno? */
state->seg.ws_file = -1;
}
/*
@ -865,6 +868,27 @@ XLogDumpDisplayStats(XLogDumpConfig *config, XLogStats *stats)
total_len, "[100%]");
}
/*
* Remove temporary directory at exit, if any.
*/
static void
cleanup_tmpwal_dir_atexit(void)
{
/*
* Before calling rmtree, we must close any open file we have in the temp
* directory; else rmdir fails on Windows.
*/
if (xlogreader_state_cleanup != NULL &&
xlogreader_state_cleanup->seg.ws_file >= 0)
WALDumpCloseSegment(xlogreader_state_cleanup);
if (TmpWalSegDir != NULL)
{
rmtree(TmpWalSegDir, true);
TmpWalSegDir = NULL;
}
}
static void
usage(void)
{
@ -1383,6 +1407,14 @@ main(int argc, char **argv)
if (!xlogreader_state)
pg_fatal("out of memory while allocating a WAL reading processor");
/*
* Set up atexit cleanup of temporary directory. This must happen before
* archive_waldump.c could possibly create the temporary directory. Also
* arm the callback to cleanup the xlogreader state.
*/
atexit(cleanup_tmpwal_dir_atexit);
xlogreader_state_cleanup = xlogreader_state;
/* first find a valid recptr to start from */
first_record = XLogFindNextRecord(xlogreader_state, private.startptr, &errormsg);
@ -1495,6 +1527,12 @@ main(int argc, char **argv)
LSN_FORMAT_ARGS(xlogreader_state->ReadRecPtr),
errormsg);
/*
* Disarm atexit cleanup of open WAL file; XLogReaderFree will close it,
* and we don't want the atexit callback trying to touch freed memory.
*/
xlogreader_state_cleanup = NULL;
XLogReaderFree(xlogreader_state);
if (private.archive_name)