Add sync of parent directory when creating or renaming a file.

third_party/leveldatabase/env_chromium.cc

Index: third_party/leveldatabase/env_chromium.cc
diff --git a/third_party/leveldatabase/env_chromium.cc b/third_party/leveldatabase/env_chromium.cc
index 3088786f61b0e5c98b2618a22dc4d832c722689b..36ac793fff1c0f9bf3c1fc07adfaae2433028f42 100644
--- a/third_party/leveldatabase/env_chromium.cc
+++ b/third_party/leveldatabase/env_chromium.cc
@@ -30,6 +30,10 @@
 #include "base/win/win_util.h"
 #endif
 
+#if !defined(OS_WIN)
+#include <fcntl.h>

[dgrogan, 2012/12/06 23:17:38]

Oh yeah, what's this for?

[ericu, 2012/12/10 18:31:12]

O_WRONLY

+#endif
+
 namespace {
 
 #if defined(OS_MACOSX) || defined(OS_WIN) || defined(OS_ANDROID) || \
@@ -66,19 +70,8 @@ FILE* fopen_internal(const char* fname, const char* mode) {
   return _wfopen(UTF8ToUTF16(fname).c_str(), ASCIIToUTF16(mode).c_str());
 #else
   return fopen(fname, mode);
-#endif
 }

[dgrogan, 2012/12/06 23:15:41]

This won't compile on windows, will it?

[ericu, 2012/12/10 18:31:12]

Fixed.  However, I'm having trouble uploading a new version, possibly because
this is in third_party and I shouldn't really be editing it here.  We just need
to swap the "#endif "and the "}", lines 73 and 74.

I also had to merge out; I'll mail you the patch so you can land it.

[dgrogan, 2012/12/10 21:53:46]

Editing it here should work, env_chromium.cc lives in the chromium repository.
The files in third_party/leveldatabase and third_party/leveldatabase/port live
in chromium.  The leveldb repo is pulled in at third_party/leveldatabase/src.

-
-}  // namespace
-
-namespace leveldb {
-
-namespace {
-
-class Thread;
-
-static const ::FilePath::CharType kLevelDBTestDirectoryPrefix[]
-    = FILE_PATH_LITERAL("leveldb-test-");
+#endif
 
 ::FilePath CreateFilePath(const std::string& file_path) {
 #if defined(OS_WIN)
@@ -96,6 +89,29 @@ std::string FilePathToString(const ::FilePath& file_path) {
 #endif
 }
 
+bool sync_parent(const std::string& fname) {
+#if !defined(OS_WIN)
+  FilePath parent_dir = CreateFilePath(fname).DirName();
+  int parent_fd = open(FilePathToString(parent_dir).c_str(), O_WRONLY);
+  if (parent_fd < 0)
+    return false;
+  fsync(parent_fd);
+  close(parent_fd);
+#endif
+  return true;
+}
+
+}  // namespace
+
+namespace leveldb {
+
+namespace {
+
+class Thread;
+
+static const ::FilePath::CharType kLevelDBTestDirectoryPrefix[]
+    = FILE_PATH_LITERAL("leveldb-test-");
+
 // TODO(jorlow): This should be moved into Chromium's base.
 const char* PlatformFileErrorString(const ::base::PlatformFileError& error) {
   switch (error) {
@@ -290,6 +306,10 @@ class ChromiumEnv : public Env {
     if (f == NULL) {
       return Status::IOError(fname, strerror(errno));
     } else {
+      if (!sync_parent(fname)) {
+        fclose(f); 
+        return Status::IOError(fname, strerror(errno));
+      }
       *result = new ChromiumWritableFile(fname, f);
       return Status::OK();
     }
@@ -357,6 +377,10 @@ class ChromiumEnv : public Env {
     Status result;
     if (!::file_util::ReplaceFile(CreateFilePath(src), CreateFilePath(dst))) {
       result = Status::IOError(src, "Could not rename file.");
+    } else {
+      sync_parent(dst);

[dgrogan, 2012/12/06 23:15:41]

Was ignoring the return value an oversight or conscious choice?

[ericu, 2012/12/10 18:31:12]

Conscious choice.  There's nothing we can do about it at this point, since we've
already done the replacement, so returning an error doesn't seem useful. 
Shouting a log message to a file that's never going to be looked at seems not
terribly useful either.

+      if (src != dst)
+        sync_parent(src);
     }
     return result;
   }
@@ -430,6 +454,10 @@ class ChromiumEnv : public Env {
       *result = NULL;
       return Status::IOError(fname, strerror(errno));
     } else {
+      if (!sync_parent(fname)) {
+        fclose(f); 
+        return Status::IOError(fname, strerror(errno));
+      }
       *result = new ChromiumLogger(f);
       return Status::OK();
     }