From: Adam Simpkins Date: Thu, 1 Dec 2016 23:45:22 +0000 (-0800) Subject: add a new writeFileAtomic() function X-Git-Tag: v2016.12.05.00~4 X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=3e107c2eb8fccefd31907f434e7e4fb2b24ad002 add a new writeFileAtomic() function Summary: Add a utility function to more safely set a file's contents by writing to a temporary file first, then renaming the temporary file into place. On Linux systems where renames are atomic, this ensures that the operation either succeeds or that the old file state is left unchanged on failure. Note that unlike most of the other APIs in FileUtil.h, I intentionally made writeFileAtomic() throw an exception on failure. This is implemented using a lower-level writeFileAtomicNoThrow() version. Callers who care about the exception overhead can use the lower level version instead. (Exception overhead should be relatively low compared to the file I/O operations, though, so I suspect most users will prefer the throwing APIs.) Reviewed By: yfeldblum Differential Revision: D4253964 fbshipit-source-id: 5301e2791b82c6f90b63bb509b0411841c266705 --- diff --git a/folly/FileUtil.cpp b/folly/FileUtil.cpp index d31e8ff9..70a8e095 100644 --- a/folly/FileUtil.cpp +++ b/folly/FileUtil.cpp @@ -18,6 +18,7 @@ #include +#include #include #include #include @@ -142,4 +143,94 @@ ssize_t pwritevFull(int fd, iovec* iov, int count, off_t offset) { return wrapvFull(pwritev, fd, iov, count, offset); } +int writeFileAtomicNoThrow( + StringPiece filename, + iovec* iov, + int count, + mode_t permissions) { + // We write the data to a temporary file name first, then atomically rename + // it into place. This ensures that the file contents will always be valid, + // even if we crash or are killed partway through writing out data. + // + // Create a buffer that will contain two things: + // - A nul-terminated version of the filename + // - The temporary file name + std::vector pathBuffer; + // Note that we have to explicitly pass in the size here to make + // sure the nul byte gets included in the data. + constexpr folly::StringPiece suffix(".XXXXXX\0", 8); + pathBuffer.resize((2 * filename.size()) + 1 + suffix.size()); + // Copy in the filename and then a nul terminator + memcpy(pathBuffer.data(), filename.data(), filename.size()); + pathBuffer[filename.size()] = '\0'; + const char* const filenameCStr = pathBuffer.data(); + // Now prepare the temporary path template + char* const tempPath = pathBuffer.data() + filename.size() + 1; + memcpy(tempPath, filename.data(), filename.size()); + memcpy(tempPath + filename.size(), suffix.data(), suffix.size()); + + auto tmpFD = mkstemp(tempPath); + if (tmpFD == -1) { + return errno; + } + bool success = false; + SCOPE_EXIT { + if (tmpFD != -1) { + close(tmpFD); + } + if (!success) { + unlink(tempPath); + } + }; + + auto rc = writevFull(tmpFD, iov, count); + if (rc == -1) { + return errno; + } + + rc = fchmod(tmpFD, permissions); + if (rc == -1) { + return errno; + } + + // Close the file before renaming to make sure all data has + // been successfully written. + rc = close(tmpFD); + tmpFD = -1; + if (rc == -1) { + return errno; + } + + rc = rename(tempPath, filenameCStr); + if (rc == -1) { + return errno; + } + success = true; + return 0; +} + +void writeFileAtomic( + StringPiece filename, + iovec* iov, + int count, + mode_t permissions) { + auto rc = writeFileAtomicNoThrow(filename, iov, count, permissions); + checkPosixError(rc, "writeFileAtomic() failed to update ", filename); +} + +void writeFileAtomic(StringPiece filename, ByteRange data, mode_t permissions) { + iovec iov; + iov.iov_base = const_cast(data.data()); + iov.iov_len = data.size(); + auto rc = writeFileAtomicNoThrow(filename, &iov, 1, permissions); + checkPosixError(rc, "writeFileAtomic() failed to update ", filename); +} + +void writeFileAtomic( + StringPiece filename, + StringPiece data, + mode_t permissions) { + writeFileAtomic(filename, ByteRange(data), permissions); +} + } // namespaces diff --git a/folly/FileUtil.h b/folly/FileUtil.h index 0c23cd96..a0b919e2 100644 --- a/folly/FileUtil.h +++ b/folly/FileUtil.h @@ -95,6 +95,9 @@ ssize_t preadvFull(int fd, iovec* iov, int count, off_t offset); * Note that writevFull and pwritevFull require iov to be non-const, unlike * writev and pwritev. The contents of iov after these functions return * is unspecified. + * + * These functions return -1 on error, or the total number of bytes written + * (which is always the same as the number of requested bytes) on success. */ ssize_t writeFull(int fd, const void* buf, size_t n); ssize_t pwriteFull(int fd, const void* buf, size_t n, off_t offset); @@ -191,6 +194,10 @@ bool readFile( * * Returns: true on success or false on failure. In the latter case * errno will be set appropriately by the failing system primitive. + * + * Note that this function may leave the file in a partially written state on + * failure. Use writeFileAtomic() if you want to ensure that the existing file + * state will be unchanged on error. */ template bool writeFile(const Container& data, const char* filename, @@ -206,4 +213,41 @@ bool writeFile(const Container& data, const char* filename, return closeNoInt(fd) == 0 && ok; } +/** + * Write file contents "atomically". + * + * This writes the data to a temporary file in the destination directory, and + * then renames it to the specified path. This guarantees that the specified + * file will be replaced the the specified contents on success, or will not be + * modified on failure. + * + * Note that on platforms that do not provide atomic filesystem rename + * functionality (e.g., Windows) this behavior may not be truly atomic. + */ +void writeFileAtomic( + StringPiece filename, + iovec* iov, + int count, + mode_t permissions = 0644); +void writeFileAtomic( + StringPiece filename, + ByteRange data, + mode_t permissions = 0644); +void writeFileAtomic( + StringPiece filename, + StringPiece data, + mode_t permissions = 0644); + +/** + * A version of writeFileAtomic() that returns an errno value instead of + * throwing on error. + * + * Returns 0 on success or an errno value on error. + */ +int writeFileAtomicNoThrow( + StringPiece filename, + iovec* iov, + int count, + mode_t permissions = 0644); + } // namespaces diff --git a/folly/test/FileUtilTest.cpp b/folly/test/FileUtilTest.cpp index 26cfde95..9d5d87ba 100644 --- a/folly/test/FileUtilTest.cpp +++ b/folly/test/FileUtilTest.cpp @@ -19,9 +19,13 @@ #include #include +#if defined(__linux__) +#include +#endif #include +#include #include #include #include @@ -340,4 +344,232 @@ TEST_F(ReadFileFd, InvalidFd) { }); PLOG(INFO); } + +class WriteFileAtomic : public ::testing::Test { + protected: + WriteFileAtomic() {} + + std::set listTmpDir() const { + std::set entries; + for (auto& entry : fs::directory_iterator(tmpDir_.path())) { + entries.insert(entry.path().filename().string()); + } + return entries; + } + + std::string readData(const string& path) const { + string data; + if (!readFile(path.c_str(), data)) { + throwSystemError("failed to read ", path); + } + return data; + } + + struct stat statFile(const string& path) const { + struct stat s; + auto rc = stat(path.c_str(), &s); + checkUnixError(rc, "failed to stat() ", path); + return s; + } + + mode_t getPerms(const string& path) { + return (statFile(path).st_mode & 0777); + } + + string tmpPath(StringPiece name) { + return tmpDir_.path().string() + "/" + name.str(); + } + + void setDirPerms(mode_t mode) { + auto rc = chmod(tmpDir_.path().string().c_str(), mode); + checkUnixError(rc, "failed to set permissions on tmp dir"); + } + + TemporaryDirectory tmpDir_{"folly_file_test"}; +}; + +TEST_F(WriteFileAtomic, writeNew) { + // Call writeFileAtomic() to create a new file + auto path = tmpPath("foo"); + auto contents = StringPiece{"contents\n"}; + writeFileAtomic(path, contents); + + // The directory should contain exactly 1 file now, with the correct contents + EXPECT_EQ(set{"foo"}, listTmpDir()); + EXPECT_EQ(contents, readData(path)); + EXPECT_EQ(0644, getPerms(path)); +} + +TEST_F(WriteFileAtomic, overwrite) { + // Call writeFileAtomic() to create a new file + auto path = tmpPath("foo"); + auto contents1 = StringPiece{"contents\n"}; + writeFileAtomic(path, contents1); + + EXPECT_EQ(set{"foo"}, listTmpDir()); + EXPECT_EQ(contents1, readData(path)); + EXPECT_EQ(0644, getPerms(path)); + + // Now overwrite the file with different contents + auto contents2 = StringPiece{"testing"}; + writeFileAtomic(path, contents2); + EXPECT_EQ(set{"foo"}, listTmpDir()); + EXPECT_EQ(contents2, readData(path)); + EXPECT_EQ(0644, getPerms(path)); + + // Test overwriting with relatively large contents, and different permissions + auto contents3 = + "asdf" + string(10240, '\n') + "foobar\n" + string(10240, 'b') + "\n"; + writeFileAtomic(path, contents3, 0444); + EXPECT_EQ(set{"foo"}, listTmpDir()); + EXPECT_EQ(contents3, readData(path)); + EXPECT_EQ(0444, getPerms(path)); + + // Test overwriting with empty contents + // + // Note that the file's permissions are 0444 at this point (read-only), + // but we writeFileAtomic() should still replace it successfully. Since we + // update it with a rename we need write permissions on the parent directory, + // but not the destination file. + auto contents4 = StringPiece(""); + writeFileAtomic(path, contents4, 0400); + EXPECT_EQ(set{"foo"}, listTmpDir()); + EXPECT_EQ(contents4, readData(path)); + EXPECT_EQ(0400, getPerms(path)); +} + +TEST_F(WriteFileAtomic, directoryPermissions) { + // Test writeFileAtomic() when we do not have write permission in the target + // directory. + // + // Make the test directory read-only + setDirPerms(0555); + SCOPE_EXIT { + // Restore directory permissions before we exit, just to ensure the code + // will be able to clean up the directory. + try { + setDirPerms(0755); + } catch (const std::exception&) { + // Intentionally ignore errors here, in case an exception is already + // being thrown. + } + }; + + // writeFileAtomic() should fail, and the directory should still be empty + auto path1 = tmpPath("foo"); + auto contents = StringPiece("testing"); + EXPECT_THROW(writeFileAtomic(path1, contents), std::system_error); + EXPECT_EQ(set{}, listTmpDir()); + + // Make the directory writable again, then create the file + setDirPerms(0755); + writeFileAtomic(path1, contents, 0400); + EXPECT_EQ(contents, readData(path1)); + EXPECT_EQ(set{"foo"}, listTmpDir()); + + // Make the directory read-only again + // Creating another file now should fail and we should still have only the + // first file. + setDirPerms(0555); + EXPECT_THROW( + writeFileAtomic(tmpPath("another_file.txt"), "x\n"), std::system_error); + EXPECT_EQ(set{"foo"}, listTmpDir()); +} + +TEST_F(WriteFileAtomic, multipleFiles) { + // Test creating multiple files in the same directory + writeFileAtomic(tmpPath("foo.txt"), "foo"); + writeFileAtomic(tmpPath("bar.txt"), "bar", 0400); + writeFileAtomic(tmpPath("foo_txt"), "underscore", 0440); + writeFileAtomic(tmpPath("foo.txt2"), "foo2", 0444); + + auto expectedPaths = set{"foo.txt", "bar.txt", "foo_txt", "foo.txt2"}; + EXPECT_EQ(expectedPaths, listTmpDir()); + EXPECT_EQ("foo", readData(tmpPath("foo.txt"))); + EXPECT_EQ("bar", readData(tmpPath("bar.txt"))); + EXPECT_EQ("underscore", readData(tmpPath("foo_txt"))); + EXPECT_EQ("foo2", readData(tmpPath("foo.txt2"))); + EXPECT_EQ(0644, getPerms(tmpPath("foo.txt"))); + EXPECT_EQ(0400, getPerms(tmpPath("bar.txt"))); + EXPECT_EQ(0440, getPerms(tmpPath("foo_txt"))); + EXPECT_EQ(0444, getPerms(tmpPath("foo.txt2"))); +} }} // namespaces + +#if defined(__linux__) +namespace { +/** + * A helper class that forces our fchmod() wrapper to fail when + * an FChmodFailure object exists. + */ +class FChmodFailure { + public: + FChmodFailure() { + ++forceFailure_; + } + ~FChmodFailure() { + --forceFailure_; + } + + static bool shouldFail() { + return forceFailure_.load() > 0; + } + + private: + static std::atomic forceFailure_; +}; + +std::atomic FChmodFailure::forceFailure_{0}; +} + +// Replace the system fchmod() function with our own stub, so we can +// trigger failures in the writeFileAtomic() tests. +int fchmod(int fd, mode_t mode) { + static const auto realFunction = + reinterpret_cast(dlsym(RTLD_NEXT, "fchmod")); + // For sanity, make sure we didn't find ourself, + // since that would cause infinite recursion. + CHECK_NE(realFunction, fchmod); + + if (FChmodFailure::shouldFail()) { + errno = EINVAL; + return -1; + } + return realFunction(fd, mode); +} + +namespace folly { +namespace test { +TEST_F(WriteFileAtomic, chmodFailure) { + auto path = tmpPath("foo"); + + // Use our stubbed out fchmod() function to force a failure when setting up + // the temporary file. + // + // First try when creating the file for the first time. + { + FChmodFailure fail; + EXPECT_THROW(writeFileAtomic(path, "foobar"), std::system_error); + } + EXPECT_EQ(set{}, listTmpDir()); + + // Now create a file normally so we can overwrite it + auto contents = StringPiece("regular perms"); + writeFileAtomic(path, contents, 0600); + EXPECT_EQ(contents, readData(path)); + EXPECT_EQ(0600, getPerms(path)); + EXPECT_EQ(set{"foo"}, listTmpDir()); + + // Now try overwriting the file when forcing fchmod to fail + { + FChmodFailure fail; + EXPECT_THROW(writeFileAtomic(path, "overwrite"), std::system_error); + } + // The file should be unchanged + EXPECT_EQ(contents, readData(path)); + EXPECT_EQ(0600, getPerms(path)); + EXPECT_EQ(set{"foo"}, listTmpDir()); +} +} +} +#endif