Expose PlatformFileLock/Unlock, fix locking in Chromium's leveldb Env

The base::CreatePlatformFile() flags PLATFORM_FILE_EXCLUSIVE_READ/WRITE
are ineffective on existing files on POSIX, and can't be used to
implement the locking scheme required by leveldb.

Add PlatformFileLock/Unlock which have the correct semantics, and
use those in ChromiumEnv. Also, crib code from leveldb's env_posix.cc
to handle in-process exclusivity which is not guaranteed by
file locks.

BUG=245471

Review URL: https://codereview.chromium.org/26306003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@228036 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 64 insertions, 9 deletions

diff --git a/third_party/leveldatabase/README.chromium b/third_party/leveldatabase/README.chromium
index 32670a7..f12a0bf 100644
--- a/third_party/leveldatabase/README.chromium
+++ b/third_party/leveldatabase/README.chromium
@@ -22,3 +22,4 @@ Local Additions:
 * ChromiumEnv wraps low-level I/O calls that may be interrupted with a
   HANDLE_EINTR macro that retries the call.
 * TRACE macros/thread name for chrome://tracing diagnostics
+* Handle in-process exclusive file locks, based on src/util/env_posix.cc
diff --git a/third_party/leveldatabase/env_chromium.cc b/third_party/leveldatabase/env_chromium.cc
index 0a78b48..b297425 100644
--- a/third_party/leveldatabase/env_chromium.cc
+++ b/third_party/leveldatabase/env_chromium.cc
@@ -207,6 +207,7 @@ class ChromiumRandomAccessFile: public RandomAccessFile {
 class ChromiumFileLock : public FileLock {
  public:
   ::base::PlatformFile file_;
+  std::string name_;
 };
 
 class Retrier {
@@ -680,9 +681,7 @@ Status ChromiumEnv::LockFile(const std::string& fname, FileLock** lock) {
   Status result;
   int flags = ::base::PLATFORM_FILE_OPEN_ALWAYS |
               ::base::PLATFORM_FILE_READ |
-              ::base::PLATFORM_FILE_WRITE |
-              ::base::PLATFORM_FILE_EXCLUSIVE_READ |
-              ::base::PLATFORM_FILE_EXCLUSIVE_WRITE;
+              ::base::PLATFORM_FILE_WRITE;
   bool created;
   ::base::PlatformFileError error_code;
   ::base::PlatformFile file;
@@ -711,21 +710,55 @@ Status ChromiumEnv::LockFile(const std::string& fname, FileLock** lock) {
     result = MakeIOError(
         fname, PlatformFileErrorString(error_code), kLockFile, error_code);
     RecordOSError(kLockFile, error_code);
-  } else {
-    ChromiumFileLock* my_lock = new ChromiumFileLock;
-    my_lock->file_ = file;
-    *lock = my_lock;
+    return result;
+  }
+
+  if (!locks_.Insert(fname)) {
+    result = MakeIOError(fname, "Lock file already locked.", kLockFile);
+    ::base::ClosePlatformFile(file);
+    return result;
   }
+
+  Retrier lock_retrier = Retrier(kLockFile, this);
+  do {
+    error_code = ::base::LockPlatformFile(file);
+  } while (error_code != ::base::PLATFORM_FILE_OK &&
+           retrier.ShouldKeepTrying(error_code));
+
+  if (error_code != ::base::PLATFORM_FILE_OK) {
+    ::base::ClosePlatformFile(file);
+    locks_.Remove(fname);
+    result = MakeIOError(
+        fname, PlatformFileErrorString(error_code), kLockFile, error_code);
+    RecordOSError(kLockFile, error_code);
+    return result;
+  }
+
+  ChromiumFileLock* my_lock = new ChromiumFileLock;
+  my_lock->file_ = file;
+  my_lock->name_ = fname;
+  *lock = my_lock;
   return result;
 }
 
 Status ChromiumEnv::UnlockFile(FileLock* lock) {
   ChromiumFileLock* my_lock = reinterpret_cast<ChromiumFileLock*>(lock);
   Status result;
-  if (!::base::ClosePlatformFile(my_lock->file_)) {
-    result = MakeIOError("Could not close lock file.", "", kUnlockFile);
+
+  ::base::PlatformFileError error_code =
+      ::base::UnlockPlatformFile(my_lock->file_);
+  if (error_code != ::base::PLATFORM_FILE_OK) {
+    result =
+        MakeIOError(my_lock->name_, "Could not unlock lock file.", kUnlockFile);
+    RecordOSError(kUnlockFile, error_code);
+    ::base::ClosePlatformFile(my_lock->file_);
+  } else if (!::base::ClosePlatformFile(my_lock->file_)) {
+    result =
+        MakeIOError(my_lock->name_, "Could not close lock file.", kUnlockFile);
     RecordErrorAt(kUnlockFile);
   }
+  bool removed = locks_.Remove(my_lock->name_);
+  DCHECK(removed);
   delete my_lock;
   return result;
 }
diff --git a/third_party/leveldatabase/env_chromium.h b/third_party/leveldatabase/env_chromium.h
index b23969c..7b7b55e 100644
--- a/third_party/leveldatabase/env_chromium.h
+++ b/third_party/leveldatabase/env_chromium.h
@@ -7,6 +7,7 @@
 
 #include <deque>
 #include <map>
+#include <set>
 
 #include "base/metrics/histogram.h"
 #include "base/platform_file.h"
@@ -14,6 +15,8 @@
 #include "leveldb/env.h"
 #include "leveldb/slice.h"
 #include "leveldb/status.h"
+#include "port/port_chromium.h"
+#include "util/mutexlock.h"
 
 namespace leveldb_env {
 
@@ -158,6 +161,23 @@ class ChromiumEnv : public leveldb::Env,
   std::string name_;
 
  private:
+  // File locks may not be exclusive within a process (e.g. on POSIX). Track
+  // locks held by the ChromiumEnv to prevent access within the process.
+  class LockTable {
+   public:
+    bool Insert(const std::string& fname) {
+      leveldb::MutexLock l(&mu_);
+      return locked_files_.insert(fname).second;
+    }
+    bool Remove(const std::string& fname) {
+      leveldb::MutexLock l(&mu_);
+      return locked_files_.erase(fname) == 1;
+    }
+   private:
+    leveldb::port::Mutex mu_;
+    std::set<std::string> locked_files_;
+  };
+
   std::map<std::string, bool> needs_sync_map_;
   base::Lock map_lock_;
 
@@ -198,6 +218,7 @@ class ChromiumEnv : public leveldb::Env,
   };
   typedef std::deque<BGItem> BGQueue;
   BGQueue queue_;
+  LockTable locks_;
 };
 
 }  // namespace leveldb_env