Commit 6da3b849 authored by Chad Parry's avatar Chad Parry Committed by Facebook Github Bot 6

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
parent 2f1ac690
...@@ -84,11 +84,10 @@ struct WTCallback : public std::enable_shared_from_this<WTCallback>, ...@@ -84,11 +84,10 @@ struct WTCallback : public std::enable_shared_from_this<WTCallback>,
} // namespace } // namespace
ThreadWheelTimekeeper::ThreadWheelTimekeeper()
ThreadWheelTimekeeper::ThreadWheelTimekeeper() : : thread_([this] { eventBase_.loopForever(); }),
thread_([this]{ eventBase_.loopForever(); }), wheelTimer_(
wheelTimer_(new HHWheelTimer(&eventBase_, std::chrono::milliseconds(1))) HHWheelTimer::newTimer(&eventBase_, std::chrono::milliseconds(1))) {
{
eventBase_.waitUntilRunning(); eventBase_.waitUntilRunning();
eventBase_.runInEventBaseThread([this]{ eventBase_.runInEventBaseThread([this]{
// 15 characters max // 15 characters max
......
...@@ -216,9 +216,8 @@ TEST_F(HHWheelTimerTest, CancelTimeout) { ...@@ -216,9 +216,8 @@ TEST_F(HHWheelTimerTest, CancelTimeout) {
*/ */
TEST_F(HHWheelTimerTest, DestroyTimeoutSet) { TEST_F(HHWheelTimerTest, DestroyTimeoutSet) {
HHWheelTimer::UniquePtr t( HHWheelTimer::UniquePtr t(
new HHWheelTimer(&eventBase, milliseconds(1))); HHWheelTimer::newTimer(&eventBase, milliseconds(1)));
TestTimeout t5_1(t.get(), milliseconds(5)); TestTimeout t5_1(t.get(), milliseconds(5));
TestTimeout t5_2(t.get(), milliseconds(5)); TestTimeout t5_2(t.get(), milliseconds(5));
...@@ -455,7 +454,8 @@ TEST_F(HHWheelTimerTest, cancelAll) { ...@@ -455,7 +454,8 @@ TEST_F(HHWheelTimerTest, cancelAll) {
} }
TEST_F(HHWheelTimerTest, SharedPtr) { TEST_F(HHWheelTimerTest, SharedPtr) {
HHWheelTimer::UniquePtr t(new HHWheelTimer(&eventBase, milliseconds(1))); HHWheelTimer::UniquePtr t(
HHWheelTimer::newTimer(&eventBase, milliseconds(1)));
TestTimeout t1; TestTimeout t1;
TestTimeout t2; TestTimeout t2;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment