Merge #14501: Fix possible data race when committing block files

ef712298c3f8bc2afdad783f05080443b72b3f77 util: Check for file being NULL in DirectoryCommit (Luke Dashjr)
457490403853321d308c6ca6aaa90d6f8f29b4cf Fix possible data race when committing block files (Evan Klitzke)
220bb16cbee5b91d0bc0fcc6c71560d631295fa5 util: Introduce DirectoryCommit commit function to sync a directory (Evan Klitzke)
ce5cbaea63ad4ea78e533bdb14f47f414061ae7f util.h: Document FileCommit function (Evan Klitzke)
844d650eea3bd809884cc5dd996a388bdc58314e util: Prefer Mac-specific F_FULLSYNC over fdatasync in FileCommit (Evan Klitzke)
f6cec0bcaf560fa310853ad3fe17022602b63d5f util: Refactor FileCommit from an #if sequence nested in #else, to a sequence of #elif (Evan Klitzke)

Pull request description:

  Reviving #12696

ACKs for top commit:
  laanwj:
    Code review ACK ef712298c3f8bc2afdad783f05080443b72b3f77

Tree-SHA512: 07d650990ef4c18d645dee3f9a199a940683ad17557d79d93979a76c4e710d8d70e6eae01d1a5991494a24a7654eb7db868be0c34a31e70b2509945d95bc9cce

configure.ac

@@ -1164,13 +1164,13 @@ if test x$TARGET_OS != xwindows; then
   fi
 fi
 
-dnl LevelDB platform checks
 AC_MSG_CHECKING(for fdatasync)
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <unistd.h>]],
  [[ fdatasync(0); ]])],
  [ AC_MSG_RESULT(yes); HAVE_FDATASYNC=1 ],
  [ AC_MSG_RESULT(no); HAVE_FDATASYNC=0 ]
 )
+AC_DEFINE_UNQUOTED([HAVE_FDATASYNC], [$HAVE_FDATASYNC], [Define to 1 if fdatasync is available.])
 
 AC_MSG_CHECKING(for F_FULLFSYNC)
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <fcntl.h>]],

src/flatfile.cpp

@@ -92,6 +92,7 @@ bool FlatFileSeq::Flush(const FlatFilePos& pos, bool finalize)
         fclose(file);
         return error("%s: failed to commit file %d", __func__, pos.nFile);
     }
+    DirectoryCommit(m_dir);
 
     fclose(file);
     return true;

src/util/system.cpp

@@ -1089,27 +1089,36 @@ bool FileCommit(FILE *file)
         LogPrintf("%s: FlushFileBuffers failed: %d\n", __func__, GetLastError());
         return false;
     }
-#else
+#elif defined(MAC_OSX) && defined(F_FULLFSYNC)
-    #if defined(HAVE_FDATASYNC)
-    if (fdatasync(fileno(file)) != 0 && errno != EINVAL) { // Ignore EINVAL for filesystems that don't support sync
-        LogPrintf("%s: fdatasync failed: %d\n", __func__, errno);
-        return false;
-    }
-    #elif defined(MAC_OSX) && defined(F_FULLFSYNC)
     if (fcntl(fileno(file), F_FULLFSYNC, 0) == -1) { // Manpage says "value other than -1" is returned on success
         LogPrintf("%s: fcntl F_FULLFSYNC failed: %d\n", __func__, errno);
         return false;
     }
-    #else
+#elif HAVE_FDATASYNC
+    if (fdatasync(fileno(file)) != 0 && errno != EINVAL) { // Ignore EINVAL for filesystems that don't support sync
+        LogPrintf("%s: fdatasync failed: %d\n", __func__, errno);
+        return false;
+    }
+#else
     if (fsync(fileno(file)) != 0 && errno != EINVAL) {
         LogPrintf("%s: fsync failed: %d\n", __func__, errno);
         return false;
     }
-    #endif
 #endif
     return true;
 }
 
+void DirectoryCommit(const fs::path &dirname)
+{
+#ifndef WIN32
+    FILE* file = fsbridge::fopen(dirname, "r");
+    if (file) {
+        fsync(fileno(file));
+        fclose(file);
+    }
+#endif
+}
+
 bool TruncateFile(FILE *file, unsigned int length) {
 #if defined(WIN32)
     return _chsize(_fileno(file), length) == 0;

src/util/system.h

@@ -71,7 +71,19 @@ bool error(const char* fmt, const Args&... args)
 }
 
 void PrintExceptionContinue(const std::exception_ptr pex, const char* pszExceptionOrigin);
+
+/**
+ * Ensure file contents are fully committed to disk, using a platform-specific
+ * feature analogous to fsync().
+ */
 bool FileCommit(FILE *file);
+
+/**
+ * Sync directory contents. This is required on some environments to ensure that
+ * newly created files are committed to disk.
+ */
+void DirectoryCommit(const fs::path &dirname);
+
 bool TruncateFile(FILE *file, unsigned int length);
 int RaiseFileDescriptorLimit(int nMinFD);
 void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length);