Check Compression Context Pool Doesn't Return nullptr

Summary:
Minor improvements to the CompressionContextPool. Although it should be
impossible, this diff checks that we do not retrieve `nullptr`s out of the
stack.

I also threw in a quick refactor to not store a copy of the deleter in the
RTPDeleter.

Reviewed By: terrelln

Differential Revision: D18131791

fbshipit-source-id: eda1856720fdb86a69ee0a2a867b25bb20ebc319

folly/compression/CompressionContextPool.h

@@ -33,19 +33,17 @@ class CompressionContextPool {
    public:
     using Pool = CompressionContextPool<T, Creator, Deleter>;
 
-    explicit ReturnToPoolDeleter(Pool* pool, const Deleter& deleter)
-        : pool_(pool), deleter_(deleter) {}
+    explicit ReturnToPoolDeleter(Pool* pool) : pool_(pool) {
+      DCHECK(pool);
+    }
 
     void operator()(T* t) {
-      InternalRef ptr(t, deleter_);
-      if (pool_ != nullptr) {
-        pool_->add(std::move(ptr));
-      }
+      InternalRef ptr(t, pool_->deleter_);
+      pool_->add(std::move(ptr));
     }
 
    private:
     Pool* pool_;
-    Deleter deleter_;
   };
 
  public:
@@ -68,6 +66,10 @@ class CompressionContextPool {
     }
     auto ptr = std::move(stack->back());
     stack->pop_back();
+    if (!ptr) {
+      throw_exception<std::logic_error>(
+          "A nullptr snuck into our context pool!?!?");
+    }
     return Ref(ptr.release(), get_deleter());
   }
 
@@ -76,7 +78,7 @@ class CompressionContextPool {
   }
 
   ReturnToPoolDeleter get_deleter() {
-    return ReturnToPoolDeleter(this, deleter_);
+    return ReturnToPoolDeleter(this);
   }
 
  private:

folly/compression/test/CompressionContextPoolTest.cpp

@@ -39,6 +39,13 @@ struct FooCreator {
   }
 };
 
+struct BadFooCreator {
+  Foo* operator()() {
+    numFoos++;
+    return nullptr;
+  }
+};
+
 struct FooDeleter {
   void operator()(Foo* f) {
     numDeleted++;
@@ -47,6 +54,7 @@ struct FooDeleter {
 };
 
 using Pool = CompressionContextPool<Foo, FooCreator, FooDeleter>;
+using BadPool = CompressionContextPool<Foo, BadFooCreator, FooDeleter>;
 
 } // anonymous namespace
 
@@ -165,5 +173,10 @@ TEST_F(CompressionContextPoolTest, testMultithread) {
   EXPECT_LE(numFoos.load(), numThreads);
   EXPECT_LE(numDeleted.load(), 0);
 }
+
+TEST_F(CompressionContextPoolTest, testBadCreate) {
+  BadPool pool;
+  EXPECT_THROW(pool.get(), std::bad_alloc);
+}
 } // namespace compression
 } // namespace folly