Create a reader-writer lock implementaiton in base.

base/synchronization/rw_lock_unittest.cc

+// Copyright 2016 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 "base/synchronization/rw_lock.h"
+
+#include <stdlib.h>
+
+#include "base/compiler_specific.h"
+#include "base/macros.h"
+#include "base/threading/platform_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+
+// Basic test to make sure that *Acquire()/*Release() don't crash ----------

[danakj, 2016/05/19 23:01:37]

replace the " ----------" with "." and ditto elsewhere

[Anand Mistry (off Chromium), 2016/05/20 04:06:39]

Done.

+
+class BasicRWLockTestThread : public PlatformThread::Delegate {
+ public:
+  explicit BasicRWLockTestThread(RWLock* lock) : lock_(lock), acquired_(0) {}
+
+  void ThreadMain() override {
+    for (int i = 0; i < 10; i++) {
+      lock_->ReadAcquire();
+      acquired_++;
+      lock_->ReadRelease();
+    }
+    for (int i = 0; i < 10; i++) {
+      lock_->WriteAcquire();
+      acquired_++;
+      PlatformThread::Sleep(TimeDelta::FromMilliseconds(rand() % 20));
+      lock_->WriteRelease();
+    }
+  }
+
+  int acquired() const { return acquired_; }
+
+ private:
+  RWLock* lock_;
+  int acquired_;
+
+  DISALLOW_COPY_AND_ASSIGN(BasicRWLockTestThread);
+};
+
+TEST(RWLockTest, Basic) {
+  RWLock lock;
+  BasicRWLockTestThread thread(&lock);
+  PlatformThreadHandle handle;
+
+  ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
+
+  int acquired = 0;
+  for (int i = 0; i < 5; i++) {

[danakj, 2016/05/19 23:01:37]

I don't really get the point of this test. What is it testing?

[Anand Mistry (off Chromium), 2016/05/20 04:06:39]

It's a basic sanity test to ensure that acquisitions that contend don't do
something completely stupid. Arguably, it's not very useful. But I think it
should still be here.

Also, I copied this all from lock_unittest :P

+    lock.ReadAcquire();
+    acquired++;
+    lock.ReadRelease();
+  }
+  for (int i = 0; i < 10; i++) {
+    lock.WriteAcquire();
+    acquired++;
+    PlatformThread::Sleep(TimeDelta::FromMilliseconds(rand() % 20));
+    lock.WriteRelease();
+  }
+  for (int i = 0; i < 5; i++) {
+    lock.ReadAcquire();
+    acquired++;
+    lock.ReadRelease();
+  }
+
+  PlatformThread::Join(handle);
+
+  EXPECT_EQ(20, acquired);
+  EXPECT_GE(20, thread.acquired());
+}
+
+// Tests that reader locks allow multiple simultaneous acquisitions ---------
+
+class ReaderRWLockTestThread : public PlatformThread::Delegate {
+ public:
+  ReaderRWLockTestThread(RWLock* lock) : lock_(lock) {}
+
+  void ThreadMain() override {
+    lock_->ReadAcquire();

[danakj, 2016/05/19 23:01:37]

Use AutoReadLock

[Anand Mistry (off Chromium), 2016/05/20 04:06:39]

Done.

+    did_acquire_ = true;
+    lock_->ReadRelease();
+  }
+
+  bool did_acquire() const { return did_acquire_; }
+
+ private:
+  RWLock* lock_;
+  bool did_acquire_ = false;
+
+  DISALLOW_COPY_AND_ASSIGN(ReaderRWLockTestThread);
+};
+
+TEST(RWLockTest, ReaderTwoThreads) {
+  RWLock lock;
+
+  AutoReadLock auto_lock(lock);
+
+  ReaderRWLockTestThread thread(&lock);
+  PlatformThreadHandle handle;
+
+  ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
+  PlatformThread::Join(handle);
+  EXPECT_TRUE(thread.did_acquire());
+}
+
+// Tests that writer locks actually exclude ---------------------------------
+
+class WriterRWLockTestThread : public PlatformThread::Delegate {
+ public:
+  WriterRWLockTestThread(RWLock* lock, int* value)
+      : lock_(lock), value_(value) {}
+
+  // Static helper which can also be called from the main thread.
+  static void DoStuff(RWLock* lock, int* value) {
+    for (int i = 0; i < 40; i++) {
+      lock->WriteAcquire();
+      int v = *value;
+      PlatformThread::Sleep(TimeDelta::FromMilliseconds(rand() % 10));
+      *value = v + 1;
+      lock->WriteRelease();
+    }
+  }
+
+  void ThreadMain() override { DoStuff(lock_, value_); }
+
+ private:
+  RWLock* lock_;
+  int* value_;
+
+  DISALLOW_COPY_AND_ASSIGN(WriterRWLockTestThread);
+};
+
+TEST(RWLockTest, MutexTwoThreads) {
+  RWLock lock;
+  int value = 0;
+
+  WriterRWLockTestThread thread(&lock, &value);
+  PlatformThreadHandle handle;
+
+  ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
+
+  WriterRWLockTestThread::DoStuff(&lock, &value);

[danakj, 2016/05/19 23:01:37]

This seems like you could write a bit more obvious test something like:

- Thread 1 takes a read lock.
- Thread 2 tries to take a write lock and change the value.
- Thread 1 waits a bit of time, checks to see the value hasn't changed.
- Thread 1 releases the read lock, waits for signal from the thread 2.
- Thread 1 sees the value has changed.

wdyt? This looping stuff seems a bit weird to me.

You could then add a thread 3 that also does a read lock and ensure it's able to
grab it before releasing the read lock on thread 1.

[Anand Mistry (off Chromium), 2016/05/20 04:06:38]

Done.
This is actually not valid. Windows doesn't support fairness at all, and posix
only does it when using real-time scheduling policies. Ideally, I'd like to have
a write-preferring implementation to prevent writers from being starved (since
writes should be rare). But that would require a custom implementation and not
rely on windows/posix.

+
+  PlatformThread::Join(handle);
+
+  EXPECT_EQ(2 * 40, value);
+}
+
+TEST(RWLockTest, MutexFourThreads) {
+  RWLock lock;
+  int value = 0;
+
+  WriterRWLockTestThread thread1(&lock, &value);
+  WriterRWLockTestThread thread2(&lock, &value);
+  WriterRWLockTestThread thread3(&lock, &value);
+  PlatformThreadHandle handle1;
+  PlatformThreadHandle handle2;
+  PlatformThreadHandle handle3;
+
+  ASSERT_TRUE(PlatformThread::Create(0, &thread1, &handle1));
+  ASSERT_TRUE(PlatformThread::Create(0, &thread2, &handle2));
+  ASSERT_TRUE(PlatformThread::Create(0, &thread3, &handle3));
+
+  WriterRWLockTestThread::DoStuff(&lock, &value);
+
+  PlatformThread::Join(handle1);
+  PlatformThread::Join(handle2);
+  PlatformThread::Join(handle3);
+
+  EXPECT_EQ(4 * 40, value);
+}
+
+}  // namespace base