diff options
author | Michael Paquier <michael@paquier.xyz> | 2018-06-25 11:20:19 +0900 |
---|---|---|
committer | Michael Paquier <michael@paquier.xyz> | 2018-06-25 11:20:19 +0900 |
commit | 6eec6724ff4519ff67c9d402d65ad8a6c27e1d0e (patch) | |
tree | f69153be7ff7760fe74f48fab6f7c29b2d20c8b0 /src/backend/access/transam | |
parent | 6350dcc1f43154d404f52ab4e891ebecef0c2115 (diff) |
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
Diffstat (limited to 'src/backend/access/transam')
-rw-r--r-- | src/backend/access/transam/twophase.c | 23 | ||||
-rw-r--r-- | src/backend/access/transam/xlog.c | 7 | ||||
-rw-r--r-- | src/backend/access/transam/xlogutils.c | 4 |
3 files changed, 34 insertions, 0 deletions
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 6d82566adb5..bda746bf63d 100644 --- 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 0cb1e9981a8..d3bfe41485d 100644 --- 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 bbae733d658..40f1198d715 100644 --- 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", |