commit 0e2a3b700db35890978b6e0c3b30acab6f76e76a
author Viet-Trung Luu <viettrungluu@chromium.org> Thu Feb 19 10:27:08 2015 -0800
committer Viet-Trung Luu <viettrungluu@chromium.org> Thu Feb 19 10:27:08 2015 -0800
tree 234c27e2b2ee1da3c7f2cd222fec77464e98ae6a
parent d8c271ee4a2ebf616d4ce1d39dab69faea263bbf

Oops, calling {Producer,Consumer}CloseImplNoLock() isn't the same as calling {Producer,Consumer}Close().

In particular, we need to wake up any waiters when we go away. (We'll
need this when serializing for real also.)

R=yzshen@chromium.org

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

mojo/edk/system/data_pipe.cc

diff --git a/mojo/edk/system/data_pipe.cc b/mojo/edk/system/data_pipe.cc
index 3b71d6d..aede492 100644
--- a/mojo/edk/system/data_pipe.cc
+++ b/mojo/edk/system/data_pipe.cc
@@ -91,17 +91,7 @@
 
 void DataPipe::ProducerClose() {
   base::AutoLock locker(lock_);
-  DCHECK(producer_open_);
-  producer_open_ = false;
-  DCHECK(has_local_producer_no_lock());
-  producer_awakable_list_.reset();
-  // Not a bug, except possibly in "user" code.
-  DVLOG_IF(2, producer_in_two_phase_write_no_lock())
-      << "Producer closed with active two-phase write";
-  producer_two_phase_max_num_bytes_written_ = 0;
-  ProducerCloseImplNoLock();
-  AwakeConsumerAwakablesForStateChangeNoLock(
-      ConsumerGetHandleSignalsStateImplNoLock());
+  ProducerCloseNoLock();
 }
 
 MojoResult DataPipe::ProducerWriteData(UserPointer<const void> elements,
@@ -271,17 +261,7 @@
 
 void DataPipe::ConsumerClose() {
   base::AutoLock locker(lock_);
-  DCHECK(consumer_open_);
-  consumer_open_ = false;
-  DCHECK(has_local_consumer_no_lock());
-  consumer_awakable_list_.reset();
-  // Not a bug, except possibly in "user" code.
-  DVLOG_IF(2, consumer_in_two_phase_read_no_lock())
-      << "Consumer closed with active two-phase read";
-  consumer_two_phase_max_num_bytes_read_ = 0;
-  ConsumerCloseImplNoLock();
-  AwakeProducerAwakablesForStateChangeNoLock(
-      ProducerGetHandleSignalsStateImplNoLock());
+  ConsumerCloseNoLock();
 }
 
 MojoResult DataPipe::ConsumerReadData(UserPointer<void> elements,
@@ -501,6 +481,36 @@
   DCHECK(!consumer_awakable_list_);
 }
 
+void DataPipe::ProducerCloseNoLock() {
+  lock_.AssertAcquired();
+  DCHECK(producer_open_);
+  producer_open_ = false;
+  DCHECK(has_local_producer_no_lock());
+  producer_awakable_list_.reset();
+  // Not a bug, except possibly in "user" code.
+  DVLOG_IF(2, producer_in_two_phase_write_no_lock())
+      << "Producer closed with active two-phase write";
+  producer_two_phase_max_num_bytes_written_ = 0;
+  ProducerCloseImplNoLock();
+  AwakeConsumerAwakablesForStateChangeNoLock(
+      ConsumerGetHandleSignalsStateImplNoLock());
+}
+
+void DataPipe::ConsumerCloseNoLock() {
+  lock_.AssertAcquired();
+  DCHECK(consumer_open_);
+  consumer_open_ = false;
+  DCHECK(has_local_consumer_no_lock());
+  consumer_awakable_list_.reset();
+  // Not a bug, except possibly in "user" code.
+  DVLOG_IF(2, consumer_in_two_phase_read_no_lock())
+      << "Consumer closed with active two-phase read";
+  consumer_two_phase_max_num_bytes_read_ = 0;
+  ConsumerCloseImplNoLock();
+  AwakeProducerAwakablesForStateChangeNoLock(
+      ProducerGetHandleSignalsStateImplNoLock());
+}
+
 void DataPipe::AwakeProducerAwakablesForStateChangeNoLock(
     const HandleSignalsState& new_producer_state) {
   lock_.AssertAcquired();

mojo/edk/system/data_pipe.h

diff --git a/mojo/edk/system/data_pipe.h b/mojo/edk/system/data_pipe.h
index 39954d4..585893e 100644
--- a/mojo/edk/system/data_pipe.h
+++ b/mojo/edk/system/data_pipe.h
@@ -116,6 +116,9 @@
   friend class base::RefCountedThreadSafe<DataPipe>;
   virtual ~DataPipe();
 
+  void ProducerCloseNoLock();
+  void ConsumerCloseNoLock();
+
   virtual void ProducerCloseImplNoLock() = 0;
   // |num_bytes.Get()| will be a nonzero multiple of |element_num_bytes_|.
   virtual MojoResult ProducerWriteDataImplNoLock(

mojo/edk/system/local_data_pipe.cc

diff --git a/mojo/edk/system/local_data_pipe.cc b/mojo/edk/system/local_data_pipe.cc
index c0ee16b..6d0760b 100644
--- a/mojo/edk/system/local_data_pipe.cc
+++ b/mojo/edk/system/local_data_pipe.cc
@@ -182,7 +182,7 @@
     size_t* actual_size,
     embedder::PlatformHandleVector* platform_handles) {
   // TODO(vtl): Support serializing producer data pipe handles.
-  ProducerCloseImplNoLock();
+  ProducerCloseNoLock();
   return false;
 }
 
@@ -336,7 +336,7 @@
     size_t* actual_size,
     embedder::PlatformHandleVector* platform_handles) {
   // TODO(vtl): Support serializing consumer data pipe handles.
-  ConsumerCloseImplNoLock();
+  ConsumerCloseNoLock();
   return false;
 }