Fix memory leak in test_subscription_nominal (ros2#469)

Signed-off-by: y-okumura-isp <y-okumura@isp.co.jp>
y-okumura-isp · Jan 29, 2020 · 3d0f7c5 · 3d0f7c5
1 parent 7537f83
commit 3d0f7c5
Showing 1 changed file with 50 additions and 38 deletions.
diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp
@@ -130,56 +130,68 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription
     rcl_ret_t ret = rcl_publisher_fini(&publisher, this->node_ptr);
     EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
   });
-  rcl_subscription_t subscription = rcl_get_zero_initialized_subscription();
+
   rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options();
-  ret = rcl_subscription_init(&subscription, this->node_ptr, ts, topic, &subscription_options);
-  ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
-  OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({
-    rcl_ret_t ret = rcl_subscription_fini(&subscription, this->node_ptr);
-    EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
-  });
-  EXPECT_EQ(strcmp(rcl_subscription_get_topic_name(&subscription), expected_topic), 0);
+
+  {
+    rcl_subscription_t subscription = rcl_get_zero_initialized_subscription();
+    ret = rcl_subscription_init(&subscription, this->node_ptr, ts, topic, &subscription_options);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
+    OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({
+      rcl_ret_t ret = rcl_subscription_fini(&subscription, this->node_ptr);
+      EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
+    });
+    EXPECT_EQ(strcmp(rcl_subscription_get_topic_name(&subscription), expected_topic), 0);
+  }
 
   // Test is_valid for subscription with nullptr
   EXPECT_FALSE(rcl_subscription_is_valid(nullptr));
   rcl_reset_error();
 
   // Test is_valid for zero initialized subscription
-  subscription = rcl_get_zero_initialized_subscription();
-  EXPECT_FALSE(rcl_subscription_is_valid(&subscription));
-  rcl_reset_error();
-
-  // Check that valid subscriber is valid
-  subscription = rcl_get_zero_initialized_subscription();
-  ret = rcl_subscription_init(&subscription, this->node_ptr, ts, topic, &subscription_options);
-  EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
-  EXPECT_TRUE(rcl_subscription_is_valid(&subscription));
-  rcl_reset_error();
-
-  // TODO(wjwwood): add logic to wait for the connection to be established
-  //                probably using the count_subscriptions busy wait mechanism
-  //                until then we will sleep for a short period of time
-  std::this_thread::sleep_for(std::chrono::milliseconds(1000));
   {
-    test_msgs__msg__BasicTypes msg;
-    test_msgs__msg__BasicTypes__init(&msg);
-    msg.int64_value = 42;
-    ret = rcl_publish(&publisher, &msg, nullptr);
-    test_msgs__msg__BasicTypes__fini(&msg);
-    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
+    rcl_subscription_t subscription = rcl_get_zero_initialized_subscription();
+    EXPECT_FALSE(rcl_subscription_is_valid(&subscription));
+    rcl_reset_error();
   }
-  bool success;
-  wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success);
-  ASSERT_TRUE(success);
+
+  // Check that valid subscriber is valid
   {
-    test_msgs__msg__BasicTypes msg;
-    test_msgs__msg__BasicTypes__init(&msg);
+    rcl_subscription_t subscription = rcl_get_zero_initialized_subscription();
+    ret = rcl_subscription_init(&subscription, this->node_ptr, ts, topic, &subscription_options);
+    EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
+    EXPECT_TRUE(rcl_subscription_is_valid(&subscription));
+    rcl_reset_error();
     OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({
-      test_msgs__msg__BasicTypes__fini(&msg);
+      rcl_ret_t ret = rcl_subscription_fini(&subscription, this->node_ptr);
+      EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     });
-    ret = rcl_take(&subscription, &msg, nullptr, nullptr);
-    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
-    ASSERT_EQ(42, msg.int64_value);
+
+    // TODO(wjwwood): add logic to wait for the connection to be established
+    //                probably using the count_subscriptions busy wait mechanism
+    //                until then we will sleep for a short period of time
+    std::this_thread::sleep_for(std::chrono::milliseconds(1000));
+    {
+      test_msgs__msg__BasicTypes msg;
+      test_msgs__msg__BasicTypes__init(&msg);
+      msg.int64_value = 42;
+      ret = rcl_publish(&publisher, &msg, nullptr);
+      test_msgs__msg__BasicTypes__fini(&msg);
+      ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
+    }
+    bool success;
+    wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success);
+    ASSERT_TRUE(success);
+    {
+      test_msgs__msg__BasicTypes msg;
+      test_msgs__msg__BasicTypes__init(&msg);
+      OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({
+        test_msgs__msg__BasicTypes__fini(&msg);
+      });
+      ret = rcl_take(&subscription, &msg, nullptr, nullptr);
+      ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
+      ASSERT_EQ(42, msg.int64_value);
+    }
   }
 }