Address set of issues with errno handling

System calls mixed up in error code paths are causing two issues which
several code paths have not correctly handled:
1) For write() calls, sometimes the system may return less bytes than
what has been written without errno being set.  Some paths were careful
enough to consider that case, and assumed that errno should be set to
ENOSPC, other calls missed that.
2) errno generated by a system call is overwritten by other system calls
which may succeed once an error code path is taken, causing what is
reported to the user to be incorrect.

This patch uses the brute-force approach of correcting all those code
paths.  Some refactoring could happen in the future, but this is let as
future work, which is not targeted for back-branches anyway.

Author: Michael Paquier
Reviewed-by: Ashutosh Sharma
Discussion: https://postgr.es/m/20180622061535.GD5215@paquier.xyz

diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 8d3c861a330eccc38edc05501c06bc5924e82a37..ed7ba181c79672d849a06214b5ed1116649b37be 100644 (file)
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -1168,9 +1168,14 @@ heap_xlog_logical_rewrite(XLogReaderState *r)
        /* write out tail end of mapping file (again) */
        pgstat_report_wait_start(WAIT_EVENT_LOGICAL_REWRITE_MAPPING_WRITE);
        if (write(fd, data, len) != len)
+       {
+               /* if write didn't set errno, assume problem is no disk space */
+               if (errno == 0)
+                       errno = ENOSPC;
                ereport(ERROR,
                                (errcode_for_file_access(),
                                 errmsg("could not write to file \"%s\": %m", path)));
+       }
        pgstat_report_wait_end();
 
        /*

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 65194db70e33a24690c3c4a32bb83e2b4c346541..a9ef1b3d73cafa3ba13d7498d99a4305fd659194 100644 (file)
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1241,12 +1241,17 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
         */
        if (fstat(fd, &stat))
        {
+               int                     save_errno = errno;
+
                CloseTransientFile(fd);
                if (give_warnings)
+               {
+                       errno = save_errno;
                        ereport(WARNING,
                                        (errcode_for_file_access(),
                                         errmsg("could not stat two-phase state file \"%s\": %m",
                                                        path)));
+               }
                return NULL;
        }
 
@@ -1274,13 +1279,18 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
        pgstat_report_wait_start(WAIT_EVENT_TWOPHASE_FILE_READ);
        if (read(fd, buf, stat.st_size) != stat.st_size)
        {
+               int                     save_errno = errno;
+
                pgstat_report_wait_end();
                CloseTransientFile(fd);
                if (give_warnings)
+               {
+                       errno = save_errno;
                        ereport(WARNING,
                                        (errcode_for_file_access(),
                                         errmsg("could not read two-phase state file \"%s\": %m",
                                                        path)));
+               }
                pfree(buf);
                return NULL;
        }
@@ -1663,16 +1673,26 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
        pgstat_report_wait_start(WAIT_EVENT_TWOPHASE_FILE_WRITE);
        if (write(fd, content, len) != len)
        {
+               int                     save_errno = errno;
+
                pgstat_report_wait_end();
                CloseTransientFile(fd);
+
+               /* if write didn't set errno, assume problem is no disk space */
+               errno = save_errno ? save_errno : ENOSPC;
                ereport(ERROR,
                                (errcode_for_file_access(),
                                 errmsg("could not write two-phase state file: %m")));
        }
        if (write(fd, &statefile_crc, sizeof(pg_crc32c)) != sizeof(pg_crc32c))
        {
+               int                     save_errno = errno;
+
                pgstat_report_wait_end();
                CloseTransientFile(fd);
+
+               /* if write didn't set errno, assume problem is no disk space */
+               errno = save_errno ? save_errno : ENOSPC;
                ereport(ERROR,
                                (errcode_for_file_access(),
                                 errmsg("could not write two-phase state file: %m")));
@@ -1686,7 +1706,10 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
        pgstat_report_wait_start(WAIT_EVENT_TWOPHASE_FILE_SYNC);
        if (pg_fsync(fd) != 0)
        {
+               int                     save_errno = errno;
+
                CloseTransientFile(fd);
+               errno = save_errno;
                ereport(ERROR,
                                (errcode_for_file_access(),
                                 errmsg("could not fsync two-phase state file: %m")));

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index adbd6a2126420441d63e73c2a71a3cfacd497624..1a419aa49bfee2d0b48ec9ffe2ef0914d940d46d 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3268,7 +3268,10 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
        pgstat_report_wait_start(WAIT_EVENT_WAL_INIT_SYNC);
        if (pg_fsync(fd) != 0)
        {
+               int                     save_errno = errno;
+
                close(fd);
+               errno = save_errno;
                ereport(ERROR,
                                (errcode_for_file_access(),
                                 errmsg("could not fsync file \"%s\": %m", tmppath)));
@@ -11675,8 +11678,10 @@ retry:
        if (lseek(readFile, (off_t) readOff, SEEK_SET) < 0)
        {
                char            fname[MAXFNAMELEN];
+               int                     save_errno = errno;
 
                XLogFileName(fname, curFileTLI, readSegNo, wal_segment_size);
+               errno = save_errno;
                ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen),
                                (errcode_for_file_access(),
                                 errmsg("could not seek in log segment %s to offset %u: %m",
@@ -11688,9 +11693,11 @@ retry:
        if (read(readFile, readBuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
        {
                char            fname[MAXFNAMELEN];
+               int                     save_errno = errno;
 
                pgstat_report_wait_end();
                XLogFileName(fname, curFileTLI, readSegNo, wal_segment_size);
+               errno = save_errno;
                ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen),
                                (errcode_for_file_access(),
                                 errmsg("could not read from log segment %s, offset %u: %m",

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 52fe55e2afb5782916fdf138c5b8185f9e8df560..4ecdc9220f0dfd204dc93f7e78105e5fb60125e7 100644 (file)
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -718,9 +718,10 @@ XLogRead(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
                        if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
                        {
                                char            path[MAXPGPATH];
+                               int                     save_errno = errno;
 
                                XLogFilePath(path, tli, sendSegNo, segsize);
-
+                               errno = save_errno;
                                ereport(ERROR,
                                                (errcode_for_file_access(),
                                                 errmsg("could not seek in log segment %s to offset %u: %m",
@@ -741,9 +742,10 @@ XLogRead(char *buf, int segsize, TimeLineID tli, XLogRecPtr startptr,
                if (readbytes <= 0)
                {
                        char            path[MAXPGPATH];
+                       int                     save_errno = errno;
 
                        XLogFilePath(path, tli, sendSegNo, segsize);
-
+                       errno = save_errno;
                        ereport(ERROR,
                                        (errcode_for_file_access(),
                                         errmsg("could not read from log segment %s, offset %u, length %lu: %m",

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 5688cbe2e9ae60c2ce791514adafdb9c2e61e01d..3f1eae38a92fca5240edd72bea43dc5ff022da8f 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -495,6 +495,8 @@ perform_base_backup(basebackup_options *opt)
                        fp = AllocateFile(pathbuf, "rb");
                        if (fp == NULL)
                        {
+                               int                     save_errno = errno;
+
                                /*
                                 * Most likely reason for this is that the file was already
                                 * removed by a checkpoint, so check for that to get a better
@@ -502,6 +504,7 @@ perform_base_backup(basebackup_options *opt)
                                 */
                                CheckXLogRemoved(segno, tli);
 
+                               errno = save_errno;
                                ereport(ERROR,
                                                (errcode_for_file_access(),
                                                 errmsg("could not open file \"%s\": %m", pathbuf)));

diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index 963878a5d87cb4852e416e9c4f659c2642faae9a..3d3f6dff1b074b4c822d4ccae93dad0de925a7ae 100644 (file)
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -578,7 +578,12 @@ CheckPointReplicationOrigin(void)
        /* write magic */
        if ((write(tmpfd, &magic, sizeof(magic))) != sizeof(magic))
        {
+               int                     save_errno = errno;
+
                CloseTransientFile(tmpfd);
+
+               /* if write didn't set errno, assume problem is no disk space */
+               errno = save_errno ? save_errno : ENOSPC;
                ereport(PANIC,
                                (errcode_for_file_access(),
                                 errmsg("could not write to file \"%s\": %m",
@@ -617,7 +622,12 @@ CheckPointReplicationOrigin(void)
                if ((write(tmpfd, &disk_state, sizeof(disk_state))) !=
                        sizeof(disk_state))
                {
+                       int                     save_errno = errno;
+
                        CloseTransientFile(tmpfd);
+
+                       /* if write didn't set errno, assume problem is no disk space */
+                       errno = save_errno ? save_errno : ENOSPC;
                        ereport(PANIC,
                                        (errcode_for_file_access(),
                                         errmsg("could not write to file \"%s\": %m",
@@ -633,7 +643,12 @@ CheckPointReplicationOrigin(void)
        FIN_CRC32C(crc);
        if ((write(tmpfd, &crc, sizeof(crc))) != sizeof(crc))
        {
+               int                     save_errno = errno;
+
                CloseTransientFile(tmpfd);
+
+               /* if write didn't set errno, assume problem is no disk space */
+               errno = save_errno ? save_errno : ENOSPC;
                ereport(PANIC,
                                (errcode_for_file_access(),
                                 errmsg("could not write to file \"%s\": %m",

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index e2f59bf5808ae928e4956356edde3cf835f3d2e4..3799ad4011ea7b621699cef7ee8eba1c93f9515e 100644 (file)
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2304,7 +2304,9 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
                int                     save_errno = errno;
 
                CloseTransientFile(fd);
-               errno = save_errno;
+
+               /* if write didn't set errno, assume problem is no disk space */
+               errno = save_errno ? save_errno : ENOSPC;
                ereport(ERROR,
                                (errcode_for_file_access(),
                                 errmsg("could not write to data file for XID %u: %m",

diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 4123cdebcfdf0b7f45bc9a97a6c95cc71645d65e..2c4a1bab4b4431ef42d60220d6695b083521579a 100644 (file)
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1605,7 +1605,12 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
        pgstat_report_wait_start(WAIT_EVENT_SNAPBUILD_WRITE);
        if ((write(fd, ondisk, needed_length)) != needed_length)
        {
+               int                     save_errno = errno;
+
                CloseTransientFile(fd);
+
+               /* if write didn't set errno, assume problem is no disk space */
+               errno = save_errno ? save_errno : ENOSPC;
                ereport(ERROR,
                                (errcode_for_file_access(),
                                 errmsg("could not write to file \"%s\": %m", tmppath)));
@@ -1623,7 +1628,10 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
        pgstat_report_wait_start(WAIT_EVENT_SNAPBUILD_SYNC);
        if (pg_fsync(fd) != 0)
        {
+               int                     save_errno = errno;
+
                CloseTransientFile(fd);
+               errno = save_errno;
                ereport(ERROR,
                                (errcode_for_file_access(),
                                 errmsg("could not fsync file \"%s\": %m", tmppath)));
@@ -1708,7 +1716,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
        pgstat_report_wait_end();
        if (readBytes != SnapBuildOnDiskConstantSize)
        {
+               int                     save_errno = errno;
+
                CloseTransientFile(fd);
+               errno = save_errno;
                ereport(ERROR,
                                (errcode_for_file_access(),
                                 errmsg("could not read file \"%s\", read %d of %d: %m",
@@ -1736,7 +1747,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
        pgstat_report_wait_end();
        if (readBytes != sizeof(SnapBuild))
        {
+               int                     save_errno = errno;
+
                CloseTransientFile(fd);
+               errno = save_errno;
                ereport(ERROR,
                                (errcode_for_file_access(),
                                 errmsg("could not read file \"%s\", read %d of %d: %m",
@@ -1753,7 +1767,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
        pgstat_report_wait_end();
        if (readBytes != sz)
        {
+               int                     save_errno = errno;
+
                CloseTransientFile(fd);
+               errno = save_errno;
                ereport(ERROR,
                                (errcode_for_file_access(),
                                 errmsg("could not read file \"%s\", read %d of %d: %m",
@@ -1769,7 +1786,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
        pgstat_report_wait_end();
        if (readBytes != sz)
        {
+               int                     save_errno = errno;
+
                CloseTransientFile(fd);
+               errno = save_errno;
                ereport(ERROR,
                                (errcode_for_file_access(),
                                 errmsg("could not read file \"%s\", read %d of %d: %m",

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index e8b76b2f207c1bb7abb22eea028c1a740a85d9e7..f5927b4d1d3abe9e4e873dae29a40cebf55ae9ce 100644 (file)
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1274,7 +1274,9 @@ SaveSlotToPath(ReplicationSlot *slot, const char *dir, int elevel)
 
                pgstat_report_wait_end();
                CloseTransientFile(fd);
-               errno = save_errno;
+
+               /* if write didn't set errno, assume problem is no disk space */
+               errno = save_errno ? save_errno : ENOSPC;
                ereport(elevel,
                                (errcode_for_file_access(),
                                 errmsg("could not write to file \"%s\": %m",
@@ -1378,7 +1380,10 @@ RestoreSlotFromDisk(const char *name)
        pgstat_report_wait_start(WAIT_EVENT_REPLICATION_SLOT_RESTORE_SYNC);
        if (pg_fsync(fd) != 0)
        {
+               int                     save_errno = errno;
+
                CloseTransientFile(fd);
+               errno = save_errno;
                ereport(PANIC,
                                (errcode_for_file_access(),
                                 errmsg("could not fsync file \"%s\": %m",

diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index 267a40debbfe5968c1941031d6f2cd2f43368e3e..331d0e72757c67eaa1c0c05c9c1ed047c52b780c 100644 (file)
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -128,7 +128,11 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 
                                pg_free(zerobuf);
                                close(fd);
-                               errno = save_errno;
+
+                               /*
+                                * If write didn't set errno, assume problem is no disk space.
+                                */
+                               errno = save_errno ? save_errno : ENOSPC;
                                return NULL;
                        }
                }
@@ -442,7 +446,14 @@ tar_write_compressed_data(void *buf, size_t count, bool flush)
                        size_t          len = ZLIB_OUT_SIZE - tar_data->zp->avail_out;
 
                        if (write(tar_data->fd, tar_data->zlibOut, len) != len)
+                       {
+                               /*
+                                * If write didn't set errno, assume problem is no disk space.
+                                */
+                               if (errno == 0)
+                                       errno = ENOSPC;
                                return false;
+                       }
 
                        tar_data->zp->next_out = tar_data->zlibOut;
                        tar_data->zp->avail_out = ZLIB_OUT_SIZE;
@@ -623,7 +634,8 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
                        save_errno = errno;
                        pg_free(tar_data->currentfile);
                        tar_data->currentfile = NULL;
-                       errno = save_errno;
+                       /* if write didn't set errno, assume problem is no disk space */
+                       errno = save_errno ? save_errno : ENOSPC;
                        return NULL;
                }
        }
@@ -818,7 +830,12 @@ tar_close(Walfile f, WalCloseMethod method)
        if (!tar_data->compression)
        {
                if (write(tar_data->fd, tf->header, 512) != 512)
+               {
+                       /* if write didn't set errno, assume problem is no disk space */
+                       if (errno == 0)
+                               errno = ENOSPC;
                        return -1;
+               }
        }
 #ifdef HAVE_LIBZ
        else
@@ -884,7 +901,12 @@ tar_finish(void)
        if (!tar_data->compression)
        {
                if (write(tar_data->fd, zerobuf, sizeof(zerobuf)) != sizeof(zerobuf))
+               {
+                       /* if write didn't set errno, assume problem is no disk space */
+                       if (errno == 0)
+                               errno = ENOSPC;
                        return false;
+               }
        }
 #ifdef HAVE_LIBZ
        else
@@ -911,7 +933,15 @@ tar_finish(void)
                                size_t          len = ZLIB_OUT_SIZE - tar_data->zp->avail_out;
 
                                if (write(tar_data->fd, tar_data->zlibOut, len) != len)
+                               {
+                                       /*
+                                        * If write didn't set errno, assume problem is no disk
+                                        * space.
+                                        */
+                                       if (errno == 0)
+                                               errno = ENOSPC;
                                        return false;
+                               }
                        }
                        if (r == Z_STREAM_END)
                                break;