[cros] RlzValueStore made protected by a cross-process lock and not persisted over browser lifetime (like on Mac).

*) Moved RecursiveCrossProcessLock out of .mm file to a common _posix file.
*) Added static method to ImportantFileWriter that does blocking write on the current thread.
*) Dedicated RLZ thread gone, replaced back with shutdown-blocking worker pool.


BUG=157348,62328


Review URL: https://chromiumcodereview.appspot.com/11308196

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

9 files changed, 131 insertions, 355 deletions

diff --git a/rlz/lib/recursive_cross_process_lock_posix.cc b/rlz/lib/recursive_cross_process_lock_posix.cc
new file mode 100644
index 0000000..071bf8c
--- /dev/null
+++ b/rlz/lib/recursive_cross_process_lock_posix.cc
@@ -0,0 +1,81 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "rlz/lib/recursive_cross_process_lock_posix.h"
+
+#include <sys/file.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#include "base/file_path.h"
+#include "base/logging.h"
+#include "base/posix/eintr_wrapper.h"
+
+namespace rlz_lib {
+
+bool RecursiveCrossProcessLock::TryGetCrossProcessLock(
+    const FilePath& lock_filename) {
+  bool just_got_lock = false;
+
+  // Emulate a recursive mutex with a non-recursive one.
+  if (pthread_mutex_trylock(&recursive_lock_) == EBUSY) {
+    if (pthread_equal(pthread_self(), locking_thread_) == 0) {
+      // Some other thread has the lock, wait for it.
+      pthread_mutex_lock(&recursive_lock_);
+      CHECK(locking_thread_ == 0);
+      just_got_lock = true;
+    }
+  } else {
+    just_got_lock = true;
+  }
+
+  locking_thread_ = pthread_self();
+
+  // Try to acquire file lock.
+  if (just_got_lock) {
+    const int kMaxTimeoutMS = 5000;  // Matches Windows.
+    const int kSleepPerTryMS = 200;
+
+    CHECK(file_lock_ == -1);
+    file_lock_ = open(lock_filename.value().c_str(), O_RDWR | O_CREAT, 0666);
+    if (file_lock_ == -1) {
+      perror("open");
+      return false;
+    }
+
+    int flock_result = -1;
+    int elapsed_ms = 0;
+    while ((flock_result =
+               HANDLE_EINTR(flock(file_lock_, LOCK_EX | LOCK_NB))) == -1 &&
+           errno == EWOULDBLOCK &&
+           elapsed_ms < kMaxTimeoutMS) {
+      usleep(kSleepPerTryMS * 1000);
+      elapsed_ms += kSleepPerTryMS;
+    }
+
+    if (flock_result == -1) {
+      perror("flock");
+      ignore_result(HANDLE_EINTR(close(file_lock_)));
+      file_lock_ = -1;
+      return false;
+    }
+    return true;
+  } else {
+    return file_lock_ != -1;
+  }
+}
+
+void RecursiveCrossProcessLock::ReleaseLock() {
+  if (file_lock_ != -1) {
+    ignore_result(HANDLE_EINTR(flock(file_lock_, LOCK_UN)));
+    ignore_result(HANDLE_EINTR(close(file_lock_)));
+    file_lock_ = -1;
+  }
+
+  locking_thread_ = 0;
+  pthread_mutex_unlock(&recursive_lock_);
+}
+
+}  // namespace rlz_lib
diff --git a/rlz/lib/recursive_cross_process_lock_posix.h b/rlz/lib/recursive_cross_process_lock_posix.h
new file mode 100644
index 0000000..a39d473
--- /dev/null
+++ b/rlz/lib/recursive_cross_process_lock_posix.h
@@ -0,0 +1,43 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef RLZ_LIB_RECURSIVE_CROSS_PROCESS_LOCK_POSIX_H_
+#define RLZ_LIB_RECURSIVE_CROSS_PROCESS_LOCK_POSIX_H_
+
+#include <pthread.h>
+
+class FilePath;
+
+namespace rlz_lib {
+
+// Creating a recursive cross-process mutex on Windows is one line. On POSIX,
+// there's no primitive for that, so this lock is emulated by an in-process
+// mutex to get the recursive part, followed by a cross-process lock for the
+// cross-process part.
+// This is a struct so that it doesn't need a static initializer.
+struct RecursiveCrossProcessLock {
+  // Tries to acquire a recursive cross-process lock. Note that this _always_
+  // acquires the in-process lock (if it wasn't already acquired). The parent
+  // directory of |lock_file| must exist.
+  bool TryGetCrossProcessLock(const FilePath& lock_filename);
+
+  // Releases the lock. Should always be called, even if
+  // TryGetCrossProcessLock() returned |false|.
+  void ReleaseLock();
+
+  pthread_mutex_t recursive_lock_;
+  pthread_t locking_thread_;
+
+  int file_lock_;
+};
+
+// On Mac, PTHREAD_RECURSIVE_MUTEX_INITIALIZER doesn't exist before 10.7 and
+// is buggy on 10.7 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51906#c34),
+// so emulate recursive locking with a normal non-recursive mutex.
+#define RECURSIVE_CROSS_PROCESS_LOCK_INITIALIZER    \
+  { PTHREAD_MUTEX_INITIALIZER, 0, -1 }
+
+}  // namespace rlz_lib
+
+#endif  // RLZ_LIB_RECURSIVE_CROSS_PROCESS_LOCK_POSIX_H_
diff --git a/rlz/lib/recursive_lock.cc b/rlz/lib/recursive_lock.cc
deleted file mode 100644
index 686cf0e..0000000
--- a/rlz/lib/recursive_lock.cc
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "rlz/lib/recursive_lock.h"
-
-#include "base/logging.h"
-
-namespace rlz_lib {
-
-RecursiveLock::RecursiveLock()
-    : owner_(),
-      recursion_() {
-}
-
-RecursiveLock::~RecursiveLock() {
-}
-
-void RecursiveLock::Acquire() {
-  base::subtle::Atomic32 me = base::PlatformThread::CurrentId();
-  if (me != base::subtle::NoBarrier_Load(&owner_)) {
-    lock_.Acquire();
-    DCHECK(!recursion_);
-    DCHECK(!owner_);
-    base::subtle::NoBarrier_Store(&owner_, me);
-  }
-  ++recursion_;
-}
-
-void RecursiveLock::Release() {
-  DCHECK_EQ(base::subtle::NoBarrier_Load(&owner_),
-            base::PlatformThread::CurrentId());
-  DCHECK_GT(recursion_, 0);
-  if (!--recursion_) {
-    base::subtle::NoBarrier_Store(&owner_, 0);
-    lock_.Release();
-  }
-}
-
-}  // namespace rlz_lib
diff --git a/rlz/lib/recursive_lock.h b/rlz/lib/recursive_lock.h
deleted file mode 100644
index 43c95747..0000000
--- a/rlz/lib/recursive_lock.h
+++ /dev/null
@@ -1,34 +0,0 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef RLZ_CHROMEOS_LIB_RECURSIVE_LOCK_H_
-#define RLZ_CHROMEOS_LIB_RECURSIVE_LOCK_H_
-
-#include "base/atomicops.h"
-#include "base/basictypes.h"
-#include "base/synchronization/lock.h"
-
-namespace rlz_lib {
-
-
-class RecursiveLock {
- public:
-  RecursiveLock();
-  ~RecursiveLock();
-
-  void Acquire();
-  void Release();
-
- private:
-  // Underlying non-recursive lock.
-  base::Lock lock_;
-  // Owner thread ID.
-  base::subtle::Atomic32 owner_;
-  // Recursion lock depth.
-  int recursion_;
-};
-
-}  // namespace rlz_lib
-
-#endif  // RLZ_CHROMEOS_LIB_RECURSIVE_LOCK_H_
diff --git a/rlz/lib/recursive_lock_unittest.cc b/rlz/lib/recursive_lock_unittest.cc
deleted file mode 100644
index 916af7f..0000000
--- a/rlz/lib/recursive_lock_unittest.cc
+++ /dev/null
@@ -1,234 +0,0 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "rlz/lib/recursive_lock.h"
-
-#include <stdlib.h>
-
-#include "base/compiler_specific.h"
-#include "base/threading/platform_thread.h"
-#include "base/time.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-using base::kNullThreadHandle;
-using base::PlatformThread;
-using base::PlatformThreadHandle;
-using base::TimeDelta;
-
-namespace rlz_lib {
-
-// Basic test to make sure that Acquire()/Release() don't crash.
-class BasicLockTestThread : public PlatformThread::Delegate {
- public:
-  BasicLockTestThread(RecursiveLock* lock) : lock_(lock), acquired_(0) {}
-
-  virtual void ThreadMain() OVERRIDE {
-    for (int i = 0; i < 10; i++) {
-      lock_->Acquire();
-      acquired_++;
-      lock_->Release();
-    }
-    for (int i = 0; i < 10; i++) {
-      lock_->Acquire();
-      acquired_++;
-      PlatformThread::Sleep(TimeDelta::FromMilliseconds(rand() % 20));
-      lock_->Release();
-    }
-  }
-
-  int acquired() const { return acquired_; }
-
- private:
-  RecursiveLock* lock_;
-  int acquired_;
-
-  DISALLOW_COPY_AND_ASSIGN(BasicLockTestThread);
-};
-
-TEST(RecursiveLockTest, Basic) {
-  RecursiveLock lock;
-  BasicLockTestThread thread(&lock);
-  PlatformThreadHandle handle = kNullThreadHandle;
-
-  ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
-
-  int acquired = 0;
-  for (int i = 0; i < 5; i++) {
-    lock.Acquire();
-    acquired++;
-    lock.Release();
-  }
-  for (int i = 0; i < 10; i++) {
-    lock.Acquire();
-    acquired++;
-    PlatformThread::Sleep(TimeDelta::FromMilliseconds(rand() % 20));
-    lock.Release();
-  }
-  for (int i = 0; i < 5; i++) {
-    lock.Acquire();
-    acquired++;
-    PlatformThread::Sleep(TimeDelta::FromMilliseconds(rand() % 20));
-    lock.Release();
-  }
-
-  PlatformThread::Join(handle);
-
-  EXPECT_EQ(acquired, 20);
-  EXPECT_EQ(thread.acquired(), 20);
-}
-
-// Tests that locks are actually exclusive.
-class MutexLockTestThread : public PlatformThread::Delegate {
- public:
-  MutexLockTestThread(RecursiveLock* lock, int* value)
-      : lock_(lock),
-        value_(value) {
-  }
-
-  // Static helper which can also be called from the main thread.
-  static void DoStuff(RecursiveLock* lock, int* value) {
-    for (int i = 0; i < 40; i++) {
-      lock->Acquire();
-      int v = *value;
-      PlatformThread::Sleep(TimeDelta::FromMilliseconds(rand() % 10));
-      *value = v + 1;
-      lock->Release();
-    }
-  }
-
-  virtual void ThreadMain() OVERRIDE {
-    DoStuff(lock_, value_);
-  }
-
- private:
-  RecursiveLock* lock_;
-  int* value_;
-
-  DISALLOW_COPY_AND_ASSIGN(MutexLockTestThread);
-};
-
-TEST(RecursiveLockTest, MutexTwoThreads) {
-  RecursiveLock lock;
-  int value = 0;
-
-  MutexLockTestThread thread(&lock, &value);
-  PlatformThreadHandle handle = kNullThreadHandle;
-
-  ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
-
-  MutexLockTestThread::DoStuff(&lock, &value);
-
-  PlatformThread::Join(handle);
-
-  EXPECT_EQ(2 * 40, value);
-}
-
-TEST(RecursiveLockTest, MutexFourThreads) {
-  RecursiveLock lock;
-  int value = 0;
-
-  MutexLockTestThread thread1(&lock, &value);
-  MutexLockTestThread thread2(&lock, &value);
-  MutexLockTestThread thread3(&lock, &value);
-  PlatformThreadHandle handle1 = kNullThreadHandle;
-  PlatformThreadHandle handle2 = kNullThreadHandle;
-  PlatformThreadHandle handle3 = kNullThreadHandle;
-
-  ASSERT_TRUE(PlatformThread::Create(0, &thread1, &handle1));
-  ASSERT_TRUE(PlatformThread::Create(0, &thread2, &handle2));
-  ASSERT_TRUE(PlatformThread::Create(0, &thread3, &handle3));
-
-  MutexLockTestThread::DoStuff(&lock, &value);
-
-  PlatformThread::Join(handle1);
-  PlatformThread::Join(handle2);
-  PlatformThread::Join(handle3);
-
-  EXPECT_EQ(4 * 40, value);
-}
-
-// Tests that locks are recursive.
-class MutexRecursiveLockTestThread : public PlatformThread::Delegate {
- public:
-  MutexRecursiveLockTestThread(RecursiveLock* lock, int* value)
-      : lock_(lock),
-        value_(value) {
-  }
-
-  // Static helper which can also be called from the main thread.
-  static void DoStuff(RecursiveLock* lock, int* value) {
-    for (int i = 0; i < 20; i++) {
-      // First lock.
-      lock->Acquire();
-      int v = *value;
-      PlatformThread::Sleep(TimeDelta::FromMilliseconds(rand() % 10));
-      *value = v + 1;
-      {
-        // Recursive lock.
-        lock->Acquire();
-        int v = *value;
-        PlatformThread::Sleep(TimeDelta::FromMilliseconds(rand() % 10));
-        *value = v + 1;
-        lock->Release();
-      }
-      v = *value;
-      PlatformThread::Sleep(TimeDelta::FromMilliseconds(rand() % 10));
-      *value = v + 1;
-      lock->Release();
-    }
-  }
-
-  virtual void ThreadMain() OVERRIDE {
-    DoStuff(lock_, value_);
-  }
-
- private:
-  RecursiveLock* lock_;
-  int* value_;
-
-  DISALLOW_COPY_AND_ASSIGN(MutexRecursiveLockTestThread);
-};
-
-
-TEST(RecursiveLockTest, MutexTwoThreadsRecursive) {
-  RecursiveLock lock;
-  int value = 0;
-
-  MutexRecursiveLockTestThread thread(&lock, &value);
-  PlatformThreadHandle handle = kNullThreadHandle;
-
-  ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
-
-  MutexRecursiveLockTestThread::DoStuff(&lock, &value);
-
-  PlatformThread::Join(handle);
-
-  EXPECT_EQ(2 * 60, value);
-}
-
-TEST(RecursiveLockTest, MutexFourThreadsRecursive) {
-  RecursiveLock lock;
-  int value = 0;
-
-  MutexRecursiveLockTestThread thread1(&lock, &value);
-  MutexRecursiveLockTestThread thread2(&lock, &value);
-  MutexRecursiveLockTestThread thread3(&lock, &value);
-  PlatformThreadHandle handle1 = kNullThreadHandle;
-  PlatformThreadHandle handle2 = kNullThreadHandle;
-  PlatformThreadHandle handle3 = kNullThreadHandle;
-
-  ASSERT_TRUE(PlatformThread::Create(0, &thread1, &handle1));
-  ASSERT_TRUE(PlatformThread::Create(0, &thread2, &handle2));
-  ASSERT_TRUE(PlatformThread::Create(0, &thread3, &handle3));
-
-  MutexRecursiveLockTestThread::DoStuff(&lock, &value);
-
-  PlatformThread::Join(handle1);
-  PlatformThread::Join(handle2);
-  PlatformThread::Join(handle3);
-
-  EXPECT_EQ(4 * 60, value);
-}
-
-}  // namespace rlz_lib
diff --git a/rlz/lib/rlz_lib.cc b/rlz/lib/rlz_lib.cc
index a820d27..8a1b729 100644
--- a/rlz/lib/rlz_lib.cc
+++ b/rlz/lib/rlz_lib.cc
@@ -16,10 +16,6 @@
 #include "rlz/lib/rlz_value_store.h"
 #include "rlz/lib/string_utils.h"
 
-#if defined(OS_CHROMEOS)
-#include "rlz/chromeos/lib/rlz_value_store_chromeos.h"
-#endif  // defined(OS_CHROMEOS)
-
 namespace {
 
 // Event information returned from ping response.
@@ -218,16 +214,6 @@ bool SetURLRequestContext(net::URLRequestContextGetter* context) {
 }
 #endif
 
-#if defined(OS_CHROMEOS)
-void RLZ_LIB_API SetIOTaskRunner(base::SequencedTaskRunner* io_task_runner) {
-  RlzValueStoreChromeOS::SetIOTaskRunner(io_task_runner);
-}
-
-void RLZ_LIB_API CleanupRlz() {
-  RlzValueStoreChromeOS::Cleanup();
-}
-#endif
-
 bool GetProductEventsAsCgi(Product product, char* cgi, size_t cgi_size) {
   if (!cgi || cgi_size <= 0) {
     ASSERT_STRING("GetProductEventsAsCgi: Invalid buffer");
diff --git a/rlz/lib/rlz_lib.h b/rlz/lib/rlz_lib.h
index 956fc9e..1f8be5c 100644
--- a/rlz/lib/rlz_lib.h
+++ b/rlz/lib/rlz_lib.h
@@ -44,12 +44,6 @@
 #endif
 #endif
 
-#if defined(OS_CHROMEOS)
-namespace base {
-class SequencedTaskRunner;
-}  // namespace base
-#endif
-
 #if defined(RLZ_NETWORK_IMPLEMENTATION_CHROME_NET)
 namespace net {
 class URLRequestContextGetter;
@@ -77,15 +71,6 @@ const size_t kMaxPingResponseLength = 0x4000;  // 16K
 bool RLZ_LIB_API SetURLRequestContext(net::URLRequestContextGetter* context);
 #endif
 
-#if defined(OS_CHROMEOS)
-// Set the MessageLoopProxy used by RLZ store to run I/O tasks on. Should be
-// called before any other API calls.
-void RLZ_LIB_API SetIOTaskRunner(base::SequencedTaskRunner* io_task_runner);
-
-// Must be invoked during shutdown to finish any remaining tasks.
-void RLZ_LIB_API CleanupRlz();
-#endif
-
 // RLZ storage functions.
 
 // Get all the events reported by this product as a CGI string to append to
diff --git a/rlz/lib/rlz_lib_test.cc b/rlz/lib/rlz_lib_test.cc
index 338473b..86e659c 100644
--- a/rlz/lib/rlz_lib_test.cc
+++ b/rlz/lib/rlz_lib_test.cc
@@ -788,7 +788,7 @@ TEST_F(RlzLibTest, BrandingWithStatefulEvents) {
   EXPECT_STREQ("events=I7S", value);
 }
 
-#if defined(OS_MACOSX)
+#if defined(OS_POSIX)
 class ReadonlyRlzDirectoryTest : public RlzLibTestNoMachineState {
  protected:
   virtual void SetUp() OVERRIDE;
@@ -799,7 +799,6 @@ void ReadonlyRlzDirectoryTest::SetUp() {
   // Make the rlz directory non-writeable.
   int chmod_result = chmod(temp_dir_.path().value().c_str(), 0500);
   ASSERT_EQ(0, chmod_result);
-
 }
 
 TEST_F(ReadonlyRlzDirectoryTest, WriteFails) {
@@ -873,7 +872,7 @@ TEST_F(RlzLibTest, ConcurrentStoreAccessWithProcessExitsWhileLockHeld) {
       rlz_lib::IE_DEFAULT_SEARCH, rlz_lib::INSTALL));
 }
 
-TEST_F(RlzLibTest, LockAcquistionSucceedsButPlistCannotBeCreated) {
+TEST_F(RlzLibTest, LockAcquistionSucceedsButStoreFileCannotBeCreated) {
   // See the comment at the top of WriteFails.
   if (!rlz_lib::SupplementaryBranding::GetBrand().empty())
     return;
@@ -881,7 +880,7 @@ TEST_F(RlzLibTest, LockAcquistionSucceedsButPlistCannotBeCreated) {
   // Create a directory where the rlz file is supposed to appear. This way,
   // the lock file can be created successfully, but creation of the rlz file
   // itself will fail.
-  int mkdir_result = mkdir(rlz_lib::testing::RlzPlistFilenameStr().c_str(),
+  int mkdir_result = mkdir(rlz_lib::testing::RlzStoreFilenameStr().c_str(),
                            0500);
   ASSERT_EQ(0, mkdir_result);
 
diff --git a/rlz/lib/rlz_value_store.h b/rlz/lib/rlz_value_store.h
index 807f100..46e87df 100644
--- a/rlz/lib/rlz_value_store.h
+++ b/rlz/lib/rlz_value_store.h
@@ -95,12 +95,7 @@ class ScopedRlzValueStoreLock {
   RlzValueStore* GetStore();
 
  private:
-#if defined(OS_WIN) || defined(OS_MACOSX)
-  // On ChromeOS, there is a singleton instance of RlzValueStore.
   scoped_ptr<RlzValueStore> store_;
-#elif defined(OS_CHROMEOS)
-  class RlzValueStoreChromeOS* store_;
-#endif
 #if defined(OS_WIN)
   LibMutex lock_;
 #elif defined(OS_MACOSX)
@@ -108,20 +103,15 @@ class ScopedRlzValueStoreLock {
 #endif
 };
 
-#if defined(OS_MACOSX) || defined(OS_CHROMEOS)
+#if defined(OS_POSIX)
 namespace testing {
 // Prefix |directory| to the path where the RLZ data file lives, for tests.
 void SetRlzStoreDirectory(const FilePath& directory);
-}  // namespace testing
-#endif  // defined(OS_MACOSX) || defined(OS_CHROMEOS)
 
-#if defined(OS_MACOSX)
-namespace testing {
-// Returns the path of the plist file used as data store.
-std::string RlzPlistFilenameStr();
+// Returns the path of the file used as data store.
+std::string RlzStoreFilenameStr();
 }  // namespace testing
-#endif  // defined(OS_MACOSX)
-
+#endif  // defined(OS_POSIX)
 
 }  // namespace rlz_lib