From 728ceba938dfadb204a4854bb76ae3b11b635401 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 2 Oct 2016 12:33:46 -0400
Subject: [PATCH] Avoid leaking FDs after an fsync failure.
Fixes errors introduced in commit bc34223bc, as detected by Coverity.
In passing, report ENOSPC for a short write while padding a new wal file in
open_walfile, make certain that close_walfile closes walfile in all cases,
and improve a couple of comments.
Michael Paquier and Tom Lane
---
src/bin/pg_basebackup/receivelog.c | 56 ++++++++++++++++++++++--------
src/common/file_utils.c | 1 +
2 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 8f29d191144..b0fa916b44b 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -86,8 +86,11 @@ mark_file_as_archived(const char *basedir, const char *fname, bool do_sync)
/*
* Open a new WAL file in the specified directory.
*
- * The file will be padded to 16Mb with zeroes. The base filename (without
- * partial_suffix) is stored in current_walfile_name.
+ * Returns true if OK; on failure, returns false after printing an error msg.
+ * On success, 'walfile' is set to the FD for the file, and the base filename
+ * (without partial_suffix) is stored in 'current_walfile_name'.
+ *
+ * The file will be padded to 16Mb with zeroes.
*/
static bool
open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
@@ -127,18 +130,23 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
}
if (statbuf.st_size == XLogSegSize)
{
- /* File is open and ready to use */
- walfile = f;
-
/*
* fsync, in case of a previous crash between padding and fsyncing the
* file.
*/
- if (stream->do_sync && fsync_fname(fn, false, progname) != 0)
- return false;
- if (stream->do_sync && fsync_parent_path(fn, progname) != 0)
- return false;
+ if (stream->do_sync)
+ {
+ if (fsync_fname(fn, false, progname) != 0 ||
+ fsync_parent_path(fn, progname) != 0)
+ {
+ /* error already printed */
+ close(f);
+ return false;
+ }
+ }
+ /* File is open and ready to use */
+ walfile = f;
return true;
}
if (statbuf.st_size != 0)
@@ -150,12 +158,20 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
return false;
}
- /* New, empty, file. So pad it to 16Mb with zeroes */
+ /*
+ * New, empty, file. So pad it to 16Mb with zeroes. If we fail partway
+ * through padding, we should attempt to unlink the file on failure, so as
+ * not to leave behind a partially-filled file.
+ */
zerobuf = pg_malloc0(XLOG_BLCKSZ);
for (bytes = 0; bytes < XLogSegSize; bytes += XLOG_BLCKSZ)
{
+ errno = 0;
if (write(f, zerobuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
{
+ /* if write didn't set errno, assume problem is no disk space */
+ if (errno == 0)
+ errno = ENOSPC;
fprintf(stderr,
_("%s: could not pad transaction log file \"%s\": %s\n"),
progname, fn, strerror(errno));
@@ -173,10 +189,16 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
* using synchronous mode, where the file is modified and fsynced
* in-place, without a directory fsync.
*/
- if (stream->do_sync && fsync_fname(fn, false, progname) != 0)
- return false;
- if (stream->do_sync && fsync_parent_path(fn, progname) != 0)
- return false;
+ if (stream->do_sync)
+ {
+ if (fsync_fname(fn, false, progname) != 0 ||
+ fsync_parent_path(fn, progname) != 0)
+ {
+ /* error already printed */
+ close(f);
+ return false;
+ }
+ }
if (lseek(f, SEEK_SET, 0) != 0)
{
@@ -186,6 +208,8 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
close(f);
return false;
}
+
+ /* File is open and ready to use */
walfile = f;
return true;
}
@@ -209,6 +233,8 @@ close_walfile(StreamCtl *stream, XLogRecPtr pos)
fprintf(stderr,
_("%s: could not determine seek position in file \"%s\": %s\n"),
progname, current_walfile_name, strerror(errno));
+ close(walfile);
+ walfile = -1;
return false;
}
@@ -216,6 +242,8 @@ close_walfile(StreamCtl *stream, XLogRecPtr pos)
{
fprintf(stderr, _("%s: could not fsync file \"%s\": %s\n"),
progname, current_walfile_name, strerror(errno));
+ close(walfile);
+ walfile = -1;
return false;
}
diff --git a/src/common/file_utils.c b/src/common/file_utils.c
index 1d855645b91..1855e2372c8 100644
--- a/src/common/file_utils.c
+++ b/src/common/file_utils.c
@@ -273,6 +273,7 @@ fsync_fname(const char *fname, bool isdir, const char *progname)
{
fprintf(stderr, _("%s: could not fsync file \"%s\": %s\n"),
progname, fname, strerror(errno));
+ (void) close(fd);
return -1;
}
--
GitLab