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[email protected]

diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index c37a6679e12112048f2a60a458fad92c33a5047e..871d9e43d772f87a302ae1b1654c84cf72d2f336 100644 (file)
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -1173,9 +1173,14 @@ heap_xlog_logical_rewrite(XLogRecPtr lsn, XLogRecord *r)
 
    /* write out tail end of mapping file (again) */
    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)));
+   }
 
    /*
     * Now fsync all previously written data. We could improve things and only

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index a7a9fc5d05949d5bf9b23fa7e0834de4e7f68013..d78ed62ea773d80639356a8e5a85c052a74c7a19 100644 (file)
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1238,12 +1238,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;
    }
 
@@ -1270,12 +1275,17 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
 
    if (read(fd, buf, stat.st_size) != stat.st_size)
    {
+       int         save_errno = errno;
+
        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;
    }
@@ -1563,14 +1573,24 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
    /* Write content and CRC */
    if (write(fd, content, len) != len)
    {
+       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 two-phase state file: %m")));
    }
    if (write(fd, &statefile_crc, sizeof(pg_crc32)) != sizeof(pg_crc32))
    {
+       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 two-phase state file: %m")));
@@ -1582,7 +1602,10 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
     */
    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 bf51a697f7d6ff9acb99d5a853894989f809292a..c50c608a1f90cef5cf2640ffc8ac4e689afc9b21 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3231,7 +3231,10 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
 
    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)));
@@ -10970,8 +10973,10 @@ retry:
    if (lseek(readFile, (off_t) readOff, SEEK_SET) < 0)
    {
        char        fname[MAXFNAMELEN];
+       int         save_errno = errno;
 
        XLogFileName(fname, curFileTLI, readSegNo);
+       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",
@@ -10982,8 +10987,10 @@ retry:
    if (read(readFile, readBuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
    {
        char        fname[MAXFNAMELEN];
+       int         save_errno = errno;
 
        XLogFileName(fname, curFileTLI, readSegNo);
+       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/replication/basebackup.c b/src/backend/replication/basebackup.c
index e4074d3035123acd6ddbb9600feceb7cf25db176..7c0b4dc9d7be663a820c1d8d23545d5057ec0190 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -448,6 +448,8 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
            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
@@ -455,6 +457,7 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
                 */
                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/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 544759b49d4fc2a6e137d4b720632c8f939a9166..ce570e44ccb8540cacb652251c10424f06284369 100644 (file)
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2170,7 +2170,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 03ca4ba6f6118f65616e562ed52c0da382b38f38..84314da0f8ad0f8eb568567429a91fa8b813cdff 100644 (file)
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1566,7 +1566,12 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
 
    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)));
@@ -1582,7 +1587,10 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
     */
    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)));
@@ -1664,7 +1672,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
    readBytes = read(fd, &ondisk, SnapBuildOnDiskConstantSize);
    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",
@@ -1690,7 +1701,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
    readBytes = read(fd, &ondisk.builder, sizeof(SnapBuild));
    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",
@@ -1705,7 +1719,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
    readBytes = read(fd, ondisk.builder.was_running.was_xip, sz);
    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",
@@ -1719,7 +1736,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
    readBytes = read(fd, ondisk.builder.committed.xip, sz);
    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 7f04b22d2d6e69eae67c7de2679100c5e4c3b0b0..661415b9a207520b5fc82e0150e65d549921f080 100644 (file)
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1031,7 +1031,9 @@ SaveSlotToPath(ReplicationSlot *slot, const char *dir, int elevel)
        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(elevel,
                (errcode_for_file_access(),
                 errmsg("could not write to file \"%s\": %m",
@@ -1134,7 +1136,10 @@ RestoreSlotFromDisk(const char *name)
     */
    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/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 33ecf20ed45b72124ce728830cd543a00ce49665..da76d9befdad52e6047188f9dc43bbc80d29717d 100644 (file)
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -139,6 +139,9 @@ open_walfile(XLogRecPtr startpoint, uint32 timeline, char *basedir,
    {
        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));
@@ -325,7 +328,9 @@ writeTimeLineHistoryFile(char *basedir, TimeLineID tli, char *filename,
         */
        close(fd);
        unlink(tmppath);
-       errno = save_errno;
+
+       /* if write didn't set errno, assume problem is no disk space */
+       errno = save_errno ? save_errno : ENOSPC;
 
        fprintf(stderr, _("%s: could not write timeline history file \"%s\": %s\n"),
                progname, tmppath, strerror(errno));
@@ -334,7 +339,10 @@ writeTimeLineHistoryFile(char *basedir, TimeLineID tli, char *filename,
 
    if (fsync(fd) != 0)
    {
+       int         save_errno = errno;
+
        close(fd);
+       errno = save_errno;
        fprintf(stderr, _("%s: could not fsync file \"%s\": %s\n"),
                progname, tmppath, strerror(errno));
        return false;
@@ -1090,6 +1098,12 @@ HandleCopyStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline,
                          copybuf + hdr_len + bytes_written,
                          bytes_to_write) != bytes_to_write)
                {
+                   /*
+                    * If write didn't set errno, assume problem is no disk
+                    * space.
+                    */
+                   if (errno == 0)
+                       errno = ENOSPC;
                    fprintf(stderr,
                            _("%s: could not write %u bytes to WAL file \"%s\": %s\n"),
                            progname, bytes_to_write, current_walfile_name,