From e1822c55a928d4bc3a3ba4bddcfdfe20062d8d34 Mon Sep 17 00:00:00 2001 From: Michael Curtiss Date: Mon, 4 Jun 2012 22:16:44 -0700 Subject: [PATCH] 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 --- folly/ProducerConsumerQueue.h | 8 ++++++-- folly/test/ProducerConsumerQueueTest.cpp | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/folly/ProducerConsumerQueue.h b/folly/ProducerConsumerQueue.h index de0e3cc0..d5d41aaf 100644 --- a/folly/ProducerConsumerQueue.h +++ b/folly/ProducerConsumerQueue.h @@ -38,7 +38,11 @@ template 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(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 bf28dcb4..06030481 100644 --- 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 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)); +} -- 2.34.1