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 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;