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 680c529580f08db5de0a040d738d9c18206d0812..601efa089d0e187fda896d9113545f45bb28dd2c 100644 (file)
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -1169,9 +1169,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 6d82566adb5ebea52fb772ccd0aeba928d9db456..bda746bf63dfdd93d9027c7d0f0f885fc15e630b 100644 (file)
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1214,12 +1214,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;
    }
 
@@ -1247,13 +1252,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;
    }
@@ -1597,16 +1607,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")));
@@ -1620,7 +1640,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 0cb1e9981a8b674e38523119dcb43b9ba701686e..d3bfe41485d54004d19bc245a0961cd5cfc3d5b5 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3243,7 +3243,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)));
@@ -11590,8 +11593,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",
@@ -11603,9 +11608,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);
+       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 bbae733d658be6ebed17ff8e0e844a069ccf35ea..40f1198d715326d0818b9da2a2a361b9bb25848c 100644 (file)
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -716,9 +716,11 @@ XLogRead(char *buf, TimeLineID tli, XLogRecPtr startptr, Size count)
            if (lseek(sendFile, (off_t) startoff, SEEK_SET) < 0)
            {
                char        path[MAXPGPATH];
+               int         save_errno = errno;
 
                XLogFilePath(path, tli, sendSegNo);
 
+               errno = save_errno;
                ereport(ERROR,
                        (errcode_for_file_access(),
                         errmsg("could not seek in log segment %s to offset %u: %m",
@@ -739,9 +741,11 @@ XLogRead(char *buf, TimeLineID tli, XLogRecPtr startptr, Size count)
        if (readbytes <= 0)
        {
            char        path[MAXPGPATH];
+           int         save_errno = errno;
 
            XLogFilePath(path, tli, sendSegNo);
 
+           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 4a98eb9bdc0337029b5d47c8844eca69ab800e07..ba4937bbd76931962143534078e835d9b82de34b 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -463,6 +463,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
@@ -470,6 +472,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/origin.c b/src/backend/replication/logical/origin.c
index 88fa35a9625d8bbf7f270c97ef36650713591b38..6a1d8a63d92f4700a915b7f726cbf892926c6abd 100644 (file)
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -579,7 +579,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",
@@ -618,7 +623,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",
@@ -634,7 +644,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 9e5720756feb616edf31190e216a02f2bbe5c9b0..4c6035634666a0970634fb19df81bb7b8cd7f0f8 100644 (file)
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2309,7 +2309,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 fba57a0470caed0156e62d22d08fae0e36808704..506e7790785a150bf3f177722ac84d2c7a0f5aee 100644 (file)
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1606,7 +1606,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)));
@@ -1624,7 +1629,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)));
@@ -1709,7 +1717,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",
@@ -1737,7 +1748,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",
@@ -1754,7 +1768,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",
@@ -1770,7 +1787,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 a8a16f55e98747b61a480f1bfd307cb0f2f34b36..cf5baf686ee8dc205b22cd794e9f968b522f1f96 100644 (file)
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1268,7 +1268,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",
@@ -1372,7 +1374,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 02d368b2427c7696286d6c68c712fa7079c5c817..a48aacf7661e76fd5446e3c1303d00fc7fbe3f41 100644 (file)
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -127,7 +127,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;
            }
        }
@@ -441,7 +445,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;
@@ -621,7 +632,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;
        }
    }
@@ -816,7 +828,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
@@ -882,7 +899,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
@@ -909,7 +931,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;