From: Daniel Pittman <dpittman@fb.com>
Date: Tue, 31 Mar 2015 20:33:48 +0000 (-0700)
Subject: MPMCQueue - FPE to invalid_argument for 0 capacity
X-Git-Tag: v0.33.0~14
X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=13dda79bc84d292539fbcb8608d2784838a40ffc;p=folly.git

MPMCQueue - FPE to invalid_argument for 0 capacity

Summary:
If an MPMCQueue was initialized with an explicit zero capacity, it would
try to compute stride for zero and trigger SIGFPE which pretty fatally
killed the process -- but in a way that required gdb to figure out the
root cause.

This replaces that with a std::invalid_argument exception, which
includes a description of the problem and should make fixing this user
error much faster.

Test Plan:
GTEST unit test added to verify behaviour; if this didn't work it would
SIGFPE and abort the test script, which is probably a good indicator
that something went wrong. :)

fbconfig -r folly && fbmake runtests

Reviewed By: ngbronson@fb.com

Subscribers: chalfant, folly-diffs@, yfeldblum, rkomorn

FB internal diff: D1930978

Signature: t1:1930978:1427740315:cc06a8b9f3c314b956ae41f813b2f904d3e979c9
---

diff --git a/folly/MPMCQueue.h b/folly/MPMCQueue.h
index 46048be1..45c4ef74 100644
--- a/folly/MPMCQueue.h
+++ b/folly/MPMCQueue.h
@@ -107,14 +107,21 @@ class MPMCQueue : boost::noncopyable {
 
   explicit MPMCQueue(size_t queueCapacity)
     : capacity_(queueCapacity)
-    , slots_(new detail::SingleElementQueue<T,Atom>[queueCapacity +
-                                                    2 * kSlotPadding])
-    , stride_(computeStride(queueCapacity))
     , pushTicket_(0)
     , popTicket_(0)
     , pushSpinCutoff_(0)
     , popSpinCutoff_(0)
   {
+    if (queueCapacity == 0)
+      throw std::invalid_argument(
+        "MPMCQueue with explicit capacity 0 is impossible"
+      );
+
+    // would sigfpe if capacity is 0
+    stride_ = computeStride(queueCapacity);
+    slots_ = new detail::SingleElementQueue<T,Atom>[queueCapacity +
+                                                    2 * kSlotPadding];
+
     // ideally this would be a static assert, but g++ doesn't allow it
     assert(alignof(MPMCQueue<T,Atom>)
            >= detail::CacheLocality::kFalseSharingRange);
diff --git a/folly/test/MPMCQueueTest.cpp b/folly/test/MPMCQueueTest.cpp
index e6042111..e606a24e 100644
--- a/folly/test/MPMCQueueTest.cpp
+++ b/folly/test/MPMCQueueTest.cpp
@@ -711,6 +711,11 @@ TEST(MPMCQueue, queue_moving) {
   LIFECYCLE_STEP(DESTRUCTOR);
 }
 
+TEST(MPMCQueue, explicit_zero_capacity_fail) {
+  ASSERT_THROW(MPMCQueue<int> cq(0), std::invalid_argument);
+}
+
+
 int main(int argc, char ** argv) {
   testing::InitGoogleTest(&argc, argv);
   gflags::ParseCommandLineFlags(&argc, &argv, true);