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

third_party/leveldatabase/env_chromium.cc

 // Copyright (c) 2011 The LevelDB Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
 
 #include <deque>
 #include <errno.h>
 #include <stdio.h>
 #include "base/at_exit.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
@@ skipping 12 matching lines @@
 #include "leveldb/env.h"
 #include "leveldb/slice.h"
 #include "port/port.h"
 #include "util/logging.h"
 
 #if defined(OS_WIN)
 #include <io.h>
 #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) || \
     defined(OS_OPENBSD)
 // The following are glibc-specific
 
 size_t fread_unlocked(void *ptr, size_t size, size_t n, FILE *file) {
   return fread(ptr, size, n, file);
 }
 
@@ skipping 16 matching lines @@
 #endif
 
 #endif
 
 // Wide-char safe fopen wrapper.
 FILE* fopen_internal(const char* fname, const char* mode) {
 #if defined(OS_WIN)
   return _wfopen(UTF8ToUTF16(fname).c_str(), ASCIIToUTF16(mode).c_str());
 #else
   return fopen(fname, mode);
+}

[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.

 #endif
-}
-
-}  // namespace
-
-namespace leveldb {
-
-namespace {
-
-class Thread;
-
-static const ::FilePath::CharType kLevelDBTestDirectoryPrefix[]
-    = FILE_PATH_LITERAL("leveldb-test-");
 
 ::FilePath CreateFilePath(const std::string& file_path) {
 #if defined(OS_WIN)
   return FilePath(UTF8ToUTF16(file_path));
 #else
   return FilePath(file_path);
 #endif
 }
 
 std::string FilePathToString(const ::FilePath& file_path) {
 #if defined(OS_WIN)
   return UTF16ToUTF8(file_path.value());
 #else
   return file_path.value();
 #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) {
     case ::base::PLATFORM_FILE_ERROR_FAILED:
       return "Opening file failed.";
     case ::base::PLATFORM_FILE_ERROR_IN_USE:
       return "File currently in use.";
     case ::base::PLATFORM_FILE_ERROR_EXISTS:
       return "File already exists.";
     case ::base::PLATFORM_FILE_ERROR_NOT_FOUND:
@@ skipping 174 matching lines @@
     return Status::OK();
   }
 
   virtual Status NewWritableFile(const std::string& fname,
                                  WritableFile** result) {
     *result = NULL;
     FILE* f = fopen_internal(fname.c_str(), "wb");
     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();
     }
   }
 
   virtual bool FileExists(const std::string& fname) {
     return ::file_util::PathExists(CreateFilePath(fname));
   }
 
   virtual Status GetChildren(const std::string& dir,
@@ skipping 47 matching lines @@
     } else {
       *size = static_cast<uint64_t>(signed_size);
     }
     return s;
   }
 
   virtual Status RenameFile(const std::string& src, const std::string& dst) {
     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;
   }
 
   virtual Status LockFile(const std::string& fname, FileLock** lock) {
     *lock = NULL;
     Status result;
     int flags = ::base::PLATFORM_FILE_OPEN_ALWAYS |
                 ::base::PLATFORM_FILE_READ |
                 ::base::PLATFORM_FILE_WRITE |
@@ skipping 53 matching lines @@
     mu_.Release();
     return Status::OK();
   }
 
   virtual Status NewLogger(const std::string& fname, Logger** result) {
     FILE* f = fopen_internal(fname.c_str(), "w");
     if (f == NULL) {
       *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();
     }
   }
 
   virtual uint64_t NowMicros() {
     return ::base::TimeTicks::Now().ToInternalValue();
   }
 
   virtual void SleepForMicroseconds(int micros) {
@@ skipping 97 matching lines @@
 ::base::LazyInstance<ChromiumEnv>::Leaky
     default_env = LAZY_INSTANCE_INITIALIZER;
 
 }
 
 Env* Env::Default() {
   return default_env.Pointer();
 }
 
 }