Merge pull request grpc#9889 from y-zeng/channel_arg

Destroy pointer args when destructing a ChannelArguments
mohits · Mar 17, 2017 · 40a947e · 40a947e
2 parents 289bc21 + 4564b8c
commit 40a947e
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 9 deletions.
diff --git a/include/grpc++/support/channel_arguments.h b/include/grpc++/support/channel_arguments.h
@@ -54,7 +54,7 @@ class ResourceQuota;
 class ChannelArguments {
  public:
   ChannelArguments();
-  ~ChannelArguments() {}
+  ~ChannelArguments();
 
   ChannelArguments(const ChannelArguments& other);
   ChannelArguments& operator=(ChannelArguments other) {

diff --git a/src/cpp/common/channel_arguments.cc b/src/cpp/common/channel_arguments.cc
@@ -81,6 +81,16 @@ ChannelArguments::ChannelArguments(const ChannelArguments& other)
   }
 }
 
+ChannelArguments::~ChannelArguments() {
+  grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
+  for (auto it = args_.begin(); it != args_.end(); ++it) {
+    if (it->type == GRPC_ARG_POINTER) {
+      it->value.pointer.vtable->destroy(&exec_ctx, it->value.pointer.p);
+    }
+  }
+  grpc_exec_ctx_finish(&exec_ctx);
+}
+
 void ChannelArguments::Swap(ChannelArguments& other) {
   args_.swap(other.args_);
   strings_.swap(other.strings_);
@@ -101,8 +111,10 @@ void ChannelArguments::SetSocketMutator(grpc_socket_mutator* mutator) {
   for (auto it = args_.begin(); it != args_.end(); ++it) {
     if (it->type == mutator_arg.type &&
         grpc::string(it->key) == grpc::string(mutator_arg.key)) {
+      GPR_ASSERT(!replaced);
       it->value.pointer.vtable->destroy(&exec_ctx, it->value.pointer.p);
       it->value.pointer = mutator_arg.value.pointer;
+      replaced = true;
     }
   }
   grpc_exec_ctx_finish(&exec_ctx);
@@ -185,7 +197,7 @@ void ChannelArguments::SetPointerWithVtable(
   arg.type = GRPC_ARG_POINTER;
   strings_.push_back(key);
   arg.key = const_cast<char*>(strings_.back().c_str());
-  arg.value.pointer.p = value;
+  arg.value.pointer.p = vtable->copy(value);
   arg.value.pointer.vtable = vtable;
   args_.push_back(arg);
 }

diff --git a/test/cpp/common/channel_arguments_test.cc b/test/cpp/common/channel_arguments_test.cc
@@ -230,13 +230,6 @@ TEST_F(ChannelArgumentsTest, SetSocketMutator) {
   EXPECT_TRUE(HasArg(arg1));
   // arg0 is replaced by arg1
   EXPECT_FALSE(HasArg(arg0));
-
-  // arg0 is destroyed by grpc_socket_mutator_to_arg(mutator1)
-  {
-    grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
-    arg1.value.pointer.vtable->destroy(&exec_ctx, arg1.value.pointer.p);
-    grpc_exec_ctx_finish(&exec_ctx);
-  }
 }
 
 TEST_F(ChannelArgumentsTest, SetUserAgentPrefix) {