From 78ad2ef735657ba79393ce8f3a41259c136e41ed Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Thu, 23 Aug 2018 16:21:56 -0400
Subject: [PATCH] grpc: Clean up initialization and allocation of timers in the
 test (#4226)

The management of the timer_ variable in the test class was a little funky, and looked leak-prone. This PR cleans this up a little,. Without this PR, #4180 memory-leaks on this test, I think due to overwriting timer_. This is all toward resolution of #4160

Risk Level: Low.
Testing: //test/common/config:grpc_mux_impl_test
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Joshua Marantz <jmarantz@google.com>
---
 test/common/config/grpc_mux_impl_test.cc | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/test/common/config/grpc_mux_impl_test.cc b/test/common/config/grpc_mux_impl_test.cc
index 170fc64e8956..16416f6a0a72 100644
--- a/test/common/config/grpc_mux_impl_test.cc
+++ b/test/common/config/grpc_mux_impl_test.cc
@@ -37,12 +37,6 @@ class GrpcMuxImplTest : public testing::Test {
   GrpcMuxImplTest() : async_client_(new Grpc::MockAsyncClient()), time_source_{} {}
 
   void setup() {
-    EXPECT_CALL(dispatcher_, createTimer_(_)).WillOnce(Invoke([this](Event::TimerCb timer_cb) {
-      timer_cb_ = timer_cb;
-      timer_ = new Event::MockTimer();
-      return timer_;
-    }));
-
     grpc_mux_.reset(new GrpcMuxImpl(
         local_info_, std::unique_ptr<Grpc::MockAsyncClient>(async_client_), dispatcher_,
         *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(
@@ -76,8 +70,6 @@ class GrpcMuxImplTest : public testing::Test {
   NiceMock<Event::MockDispatcher> dispatcher_;
   Runtime::MockRandomGenerator random_;
   Grpc::MockAsyncClient* async_client_;
-  Event::MockTimer* timer_;
-  Event::TimerCb timer_cb_;
   Grpc::MockAsyncStream async_stream_;
   std::unique_ptr<GrpcMuxImpl> grpc_mux_;
   NiceMock<MockGrpcMuxCallbacks> callbacks_;
@@ -107,6 +99,14 @@ TEST_F(GrpcMuxImplTest, MultipleTypeUrlStreams) {
 
 // Validate behavior when multiple type URL watches are maintained and the stream is reset.
 TEST_F(GrpcMuxImplTest, ResetStream) {
+  Event::MockTimer* timer = nullptr;
+  Event::TimerCb timer_cb;
+  EXPECT_CALL(dispatcher_, createTimer_(_)).WillOnce(Invoke([&timer, &timer_cb](Event::TimerCb cb) {
+    timer_cb = cb;
+    EXPECT_EQ(nullptr, timer);
+    timer = new Event::MockTimer();
+    return timer;
+  }));
   setup();
   InSequence s;
   auto foo_sub = grpc_mux_->subscribe("foo", {"x", "y"}, callbacks_);
@@ -119,13 +119,14 @@ TEST_F(GrpcMuxImplTest, ResetStream) {
   grpc_mux_->start();
 
   EXPECT_CALL(random_, random());
-  EXPECT_CALL(*timer_, enableTimer(_));
+  ASSERT_TRUE(timer != nullptr); // initialized from dispatcher mock.
+  EXPECT_CALL(*timer, enableTimer(_));
   grpc_mux_->onRemoteClose(Grpc::Status::GrpcStatus::Canceled, "");
   EXPECT_CALL(*async_client_, start(_, _)).WillOnce(Return(&async_stream_));
   expectSendMessage("foo", {"x", "y"}, "");
   expectSendMessage("bar", {}, "");
   expectSendMessage("baz", {"z"}, "");
-  timer_cb_();
+  timer_cb();
 
   expectSendMessage("baz", {}, "");
   expectSendMessage("foo", {}, "");