Mojo: Add debug-only checks to Waiter that check that Init() has been…

… called. (And fix a test that didn't follow the contract properly.) R=davemoore@chromium.org Review URL: https://codereview.chromium.org/28983006 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@229889 0039d316-1c4b-4281-b951-d872f2087c98
Denger-Network · Oct 21, 2013 · f1c0c49 · f1c0c49
1 parent 15d0233
commit f1c0c49
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 0 deletions.
diff --git a/mojo/system/message_pipe_unittest.cc b/mojo/system/message_pipe_unittest.cc
@@ -333,15 +333,18 @@ TEST(MessagePipeTest, BasicWaiting) {
   uint32_t buffer_size;
 
   // Always writable (until the other port is closed).
+  waiter.Init();
   EXPECT_EQ(MOJO_RESULT_ALREADY_EXISTS,
             mp->AddWaiter(0, &waiter, MOJO_WAIT_FLAG_WRITABLE, 0));
+  waiter.Init();
   EXPECT_EQ(MOJO_RESULT_ALREADY_EXISTS,
             mp->AddWaiter(0,
                           &waiter,
                           MOJO_WAIT_FLAG_READABLE | MOJO_WAIT_FLAG_WRITABLE,
                           0));
 
   // Not yet readable.
+  waiter.Init();
   EXPECT_EQ(MOJO_RESULT_OK,
             mp->AddWaiter(0, &waiter, MOJO_WAIT_FLAG_READABLE, 1));
   EXPECT_EQ(MOJO_RESULT_DEADLINE_EXCEEDED, waiter.Wait(0));
@@ -356,25 +359,30 @@ TEST(MessagePipeTest, BasicWaiting) {
                              MOJO_WRITE_MESSAGE_FLAG_NONE));
 
   // Port 1 should already be readable now.
+  waiter.Init();
   EXPECT_EQ(MOJO_RESULT_ALREADY_EXISTS,
             mp->AddWaiter(1, &waiter, MOJO_WAIT_FLAG_READABLE, 2));
+  waiter.Init();
   EXPECT_EQ(MOJO_RESULT_ALREADY_EXISTS,
             mp->AddWaiter(1,
                           &waiter,
                           MOJO_WAIT_FLAG_READABLE | MOJO_WAIT_FLAG_WRITABLE,
                           0));
   // ... and still writable.
+  waiter.Init();
   EXPECT_EQ(MOJO_RESULT_ALREADY_EXISTS,
             mp->AddWaiter(1, &waiter, MOJO_WAIT_FLAG_WRITABLE, 3));
 
   // Close port 0.
   mp->Close(0);
 
   // Now port 1 should not be writable.
+  waiter.Init();
   EXPECT_EQ(MOJO_RESULT_FAILED_PRECONDITION,
             mp->AddWaiter(1, &waiter, MOJO_WAIT_FLAG_WRITABLE, 4));
 
   // But it should still be readable.
+  waiter.Init();
   EXPECT_EQ(MOJO_RESULT_ALREADY_EXISTS,
             mp->AddWaiter(1, &waiter, MOJO_WAIT_FLAG_READABLE, 5));
 
@@ -389,6 +397,7 @@ TEST(MessagePipeTest, BasicWaiting) {
   EXPECT_EQ(123456789, buffer[0]);
 
   // Now port 1 should no longer be readable.
+  waiter.Init();
   EXPECT_EQ(MOJO_RESULT_FAILED_PRECONDITION,
             mp->AddWaiter(1, &waiter, MOJO_WAIT_FLAG_READABLE, 6));
 
@@ -406,6 +415,7 @@ TEST(MessagePipeTest, ThreadedWaiting) {
     scoped_refptr<MessagePipe> mp(new MessagePipe());
     test::SimpleWaiterThread thread(&result);
 
+    thread.waiter()->Init();
     EXPECT_EQ(MOJO_RESULT_OK,
               mp->AddWaiter(1, thread.waiter(), MOJO_WAIT_FLAG_READABLE, 0));
     thread.Start();
@@ -431,6 +441,7 @@ TEST(MessagePipeTest, ThreadedWaiting) {
     scoped_refptr<MessagePipe> mp(new MessagePipe());
     test::SimpleWaiterThread thread(&result);
 
+    thread.waiter()->Init();
     EXPECT_EQ(MOJO_RESULT_OK,
               mp->AddWaiter(1, thread.waiter(), MOJO_WAIT_FLAG_READABLE, 0));
     thread.Start();
@@ -451,6 +462,7 @@ TEST(MessagePipeTest, ThreadedWaiting) {
     scoped_refptr<MessagePipe> mp(new MessagePipe());
     test::SimpleWaiterThread thread(&result);
 
+    thread.waiter()->Init();
     EXPECT_EQ(MOJO_RESULT_OK,
               mp->AddWaiter(1, thread.waiter(), MOJO_WAIT_FLAG_READABLE, 0));
     thread.Start();

diff --git a/mojo/system/waiter.cc b/mojo/system/waiter.cc
@@ -14,6 +14,9 @@ namespace system {
 
 Waiter::Waiter()
     : cv_(&lock_),
+#ifndef NDEBUG
+      initialized_(false),
+#endif
       awoken_(false),
       wait_result_(MOJO_RESULT_INTERNAL) {
 }
@@ -22,6 +25,9 @@ Waiter::~Waiter() {
 }
 
 void Waiter::Init() {
+#ifndef NDEBUG
+  initialized_ = true;
+#endif
   awoken_ = false;
   // NOTE(vtl): If performance ever becomes an issue, we can disable the setting
   // of |wait_result_| (except the first one in |Awake()|) in Release builds.
@@ -32,6 +38,12 @@ void Waiter::Init() {
 MojoResult Waiter::Wait(MojoDeadline deadline) {
   base::AutoLock locker(lock_);
 
+#ifndef NDEBUG
+  DCHECK(initialized_);
+  // It'll need to be re-initialized after this.
+  initialized_ = false;
+#endif
+
   // Fast-path the already-awoken case:
   if (awoken_) {
     DCHECK_NE(wait_result_, MOJO_RESULT_INTERNAL);

diff --git a/mojo/system/waiter.h b/mojo/system/waiter.h
@@ -46,6 +46,9 @@ class MOJO_SYSTEM_EXPORT Waiter {
  private:
   base::ConditionVariable cv_;  // Associated to |lock_|.
   base::Lock lock_;  // Protects the following members.
+#ifndef NDEBUG
+  bool initialized_;
+#endif
   bool awoken_;
   MojoResult wait_result_;