From: Chad Parry <cparry@fb.com>
Date: Fri, 13 May 2016 18:56:03 +0000 (-0700)
Subject: Construct all HHWheelTimer instances with the factory method
X-Git-Tag: 2016.07.26~239
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=6da3b849193da48ae68bad4bab8dda171e0f1aaf;p=folly.git

Construct all HHWheelTimer instances with the factory method

Summary:
This diff standardizes `HHWheelTimer` construction. It's not that safe to allow anyone to call `new HHWheelTimer` because the caller needs to remember to call `DelayedDestruction::destroy`.

Once callers are made to be safer, I'll be able to change the `HHWheelTimer` to use standard smart pointers instead of `DelayedDestruction`.

This picks up some work that I started in D2627941 but had to postpone.

This was mostly a mechanical change:

  ( fbgs -ls 'new HHWheelTimer' ; fbgs -ls 'new folly::HHWheelTimer' ) | xargs perl -pi -e 's/\bnew\s+((?:folly::)?HHWheelTimer)\b/$1::newTimer/g'

Then I manually inspected the code. I reverted `folly/io/async/HHWheelTimer.h`, since the `newTimer` factory method is the one place that we still want to call `new HHWheelTimer`. I manually edited `proxygen/lib/utils/SharedWheelTimer.cpp`, since that was using a `shared_ptr` with a custom destructor, which isn't needed anymore. I reverted `common/io/async/d` since the D shim is meant to pass around only raw pointers. I had to replace each instance of `foo.reset(…)` with `foo = …` . Then I made manual edits to `common/clientpool/ClientPool2-inl.h` because that code wants to store the timer in a `ThreadLocalPtr`.

Reviewed By: yfeldblum

Differential Revision: D3237530

fbshipit-source-id: 96bfb6987098618ad5430c21b1314f385f04a93d
---

diff --git a/folly/futures/ThreadWheelTimekeeper.cpp b/folly/futures/ThreadWheelTimekeeper.cpp
index 0b091e6d..332e4549 100644
--- a/folly/futures/ThreadWheelTimekeeper.cpp
+++ b/folly/futures/ThreadWheelTimekeeper.cpp
@@ -84,11 +84,10 @@ struct WTCallback : public std::enable_shared_from_this<WTCallback>,
 
 } // namespace
 
-
-ThreadWheelTimekeeper::ThreadWheelTimekeeper() :
-  thread_([this]{ eventBase_.loopForever(); }),
-  wheelTimer_(new HHWheelTimer(&eventBase_, std::chrono::milliseconds(1)))
-{
+ThreadWheelTimekeeper::ThreadWheelTimekeeper()
+    : thread_([this] { eventBase_.loopForever(); }),
+      wheelTimer_(
+          HHWheelTimer::newTimer(&eventBase_, std::chrono::milliseconds(1))) {
   eventBase_.waitUntilRunning();
   eventBase_.runInEventBaseThread([this]{
     // 15 characters max
diff --git a/folly/io/async/test/HHWheelTimerTest.cpp b/folly/io/async/test/HHWheelTimerTest.cpp
index c8f5a405..df38277d 100644
--- a/folly/io/async/test/HHWheelTimerTest.cpp
+++ b/folly/io/async/test/HHWheelTimerTest.cpp
@@ -216,9 +216,8 @@ TEST_F(HHWheelTimerTest, CancelTimeout) {
  */
 
 TEST_F(HHWheelTimerTest, DestroyTimeoutSet) {
-
   HHWheelTimer::UniquePtr t(
-    new HHWheelTimer(&eventBase, milliseconds(1)));
+      HHWheelTimer::newTimer(&eventBase, milliseconds(1)));
 
   TestTimeout t5_1(t.get(), milliseconds(5));
   TestTimeout t5_2(t.get(), milliseconds(5));
@@ -455,7 +454,8 @@ TEST_F(HHWheelTimerTest, cancelAll) {
 }
 
 TEST_F(HHWheelTimerTest, SharedPtr) {
-  HHWheelTimer::UniquePtr t(new HHWheelTimer(&eventBase, milliseconds(1)));
+  HHWheelTimer::UniquePtr t(
+      HHWheelTimer::newTimer(&eventBase, milliseconds(1)));
 
   TestTimeout t1;
   TestTimeout t2;