Fix error in ProducerQueue::isEmpty

Summary:
Oops.

Also: documented the slightly confusing behavior w.r.t. 'size'.

Test Plan: Added a unit test.

Reviewers: tjackson, jdelong

Reviewed By: jdelong

CC: folly@lists, lr, bagashe
Differential Revision: https://phabricator.fb.com/D486832

diff --git a/folly/ProducerConsumerQueue.h b/folly/ProducerConsumerQueue.h
index de0e3cc0a39892cde2903ad60f560de33b8809d5..d5d41aaf8dbeae57e7958ddebe49af3581edf484 100644 (file)
--- a/folly/ProducerConsumerQueue.h
+++ b/folly/ProducerConsumerQueue.h
@@ -38,7 +38,11 @@ template<class T>
 struct ProducerConsumerQueue : private boost::noncopyable {
   typedef T value_type;
 
-  // size must be >= 2
+  // size must be >= 2.
+  //
+  // Also, note that the number of usable slots in the queue at any
+  // given time is actually (size-1), so if you start with an empty queue,
+  // isFull() will return true after size-1 insertions.
   explicit ProducerConsumerQueue(uint32_t size)
     : size_(size)
     , records_(static_cast<T*>(std::malloc(sizeof(T) * size)))
@@ -129,7 +133,7 @@ struct ProducerConsumerQueue : private boost::noncopyable {
   }
 
   bool isEmpty() const {
-   return readIndex_.load(std::memory_order_consume) !=
+   return readIndex_.load(std::memory_order_consume) ==
          writeIndex_.load(std::memory_order_consume);
   }
 

diff --git a/folly/test/ProducerConsumerQueueTest.cpp b/folly/test/ProducerConsumerQueueTest.cpp
index bf28dcb41c56115386d041f13e370581aa8a4d3c..06030481ce04d19825b062565bb0582f68674a76 100644 (file)
--- a/folly/test/ProducerConsumerQueueTest.cpp
+++ b/folly/test/ProducerConsumerQueueTest.cpp
@@ -266,3 +266,19 @@ TEST(PCQ, Destructor) {
   }
   EXPECT_EQ(DtorChecker::numInstances, 0);
 }
+
+TEST(PCQ, EmptyFull) {
+  folly::ProducerConsumerQueue<int> queue(3);
+  EXPECT_TRUE(queue.isEmpty());
+  EXPECT_FALSE(queue.isFull());
+
+  EXPECT_TRUE(queue.write(1));
+  EXPECT_FALSE(queue.isEmpty());
+  EXPECT_FALSE(queue.isFull());
+
+  EXPECT_TRUE(queue.write(2));
+  EXPECT_FALSE(queue.isEmpty());
+  EXPECT_TRUE(queue.isFull());  // Tricky: full after 2 writes, not 3.
+
+  EXPECT_FALSE(queue.write(3));
+}