From 0c4c743d29d3c313a46d1079003e49081a64e292 Mon Sep 17 00:00:00 2001 From: Cameron Evans Date: Fri, 17 May 2019 15:38:05 -0700 Subject: [PATCH] Fix leaks in test_wait.cpp (#439) Signed-off-by: cevans87 --- rcl/test/rcl/test_wait.cpp | 102 +++++++++++++++++++++++-------------- 1 file changed, 65 insertions(+), 37 deletions(-) diff --git a/rcl/test/rcl/test_wait.cpp b/rcl/test/rcl/test_wait.cpp index d61c02cd4..a5399e91b 100644 --- a/rcl/test/rcl/test_wait.cpp +++ b/rcl/test/rcl/test_wait.cpp @@ -111,35 +111,42 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), negative_timeout) { rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 1, 1, 0, 0, 0, context_ptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_wait_set_fini(&wait_set); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); // Add a dummy guard condition to avoid an error rcl_guard_condition_t guard_cond = rcl_get_zero_initialized_guard_condition(); ret = rcl_guard_condition_init( &guard_cond, this->context_ptr, rcl_guard_condition_get_default_options()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_guard_condition_fini(&guard_cond); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); ret = rcl_wait_set_add_guard_condition(&wait_set, &guard_cond, NULL); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_clock_t clock; rcl_allocator_t allocator = rcl_get_default_allocator(); ret = rcl_clock_init(RCL_STEADY_TIME, &clock, &allocator); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_clock_fini(&clock); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_timer_t timer = rcl_get_zero_initialized_timer(); ret = rcl_timer_init( &timer, &clock, this->context_ptr, RCL_MS_TO_NS(10), nullptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_timer(&wait_set, &timer, NULL); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ - rcl_ret_t ret = rcl_guard_condition_fini(&guard_cond); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_fini(&wait_set); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ret = rcl_timer_fini(&timer); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); + ret = rcl_wait_set_add_timer(&wait_set, &timer, NULL); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; int64_t timeout = -1; std::chrono::steady_clock::time_point before_sc = std::chrono::steady_clock::now(); @@ -158,21 +165,22 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), zero_timeout) { rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 1, 1, 0, 0, 0, context_ptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_wait_set_fini(&wait_set); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); // Add a dummy guard condition to avoid an error rcl_guard_condition_t guard_cond = rcl_get_zero_initialized_guard_condition(); ret = rcl_guard_condition_init( &guard_cond, this->context_ptr, rcl_guard_condition_get_default_options()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, &guard_cond, NULL); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ - rcl_ret_t ret = rcl_guard_condition_fini(&guard_cond); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_fini(&wait_set); + ret = rcl_guard_condition_fini(&guard_cond); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); + ret = rcl_wait_set_add_guard_condition(&wait_set, &guard_cond, NULL); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // Time spent during wait should be negligible. int64_t timeout = 0; @@ -191,23 +199,24 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), zero_timeout_triggered rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 1, 0, 0, 0, 0, context_ptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_wait_set_fini(&wait_set); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); rcl_guard_condition_t guard_cond = rcl_get_zero_initialized_guard_condition(); ret = rcl_guard_condition_init( &guard_cond, this->context_ptr, rcl_guard_condition_get_default_options()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_guard_condition_fini(&guard_cond); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); ret = rcl_wait_set_add_guard_condition(&wait_set, &guard_cond, NULL); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ret = rcl_trigger_guard_condition(&guard_cond); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ - rcl_ret_t ret = rcl_guard_condition_fini(&guard_cond); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_fini(&wait_set); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - }); - // Time spent during wait should be negligible. int64_t timeout = 0; std::chrono::steady_clock::time_point before_sc = std::chrono::steady_clock::now(); @@ -225,18 +234,30 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), canceled_timer) { rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 1, 1, 0, 0, 0, context_ptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_wait_set_fini(&wait_set); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); // Add a dummy guard condition to avoid an error rcl_guard_condition_t guard_cond = rcl_get_zero_initialized_guard_condition(); ret = rcl_guard_condition_init( &guard_cond, this->context_ptr, rcl_guard_condition_get_default_options()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_guard_condition_fini(&guard_cond); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); ret = rcl_wait_set_add_guard_condition(&wait_set, &guard_cond, NULL); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_clock_t clock; rcl_allocator_t allocator = rcl_get_default_allocator(); ret = rcl_clock_init(RCL_STEADY_TIME, &clock, &allocator); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_clock_fini(&clock); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_timer_t canceled_timer = rcl_get_zero_initialized_timer(); @@ -244,19 +265,14 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), canceled_timer) { &canceled_timer, &clock, this->context_ptr, RCL_MS_TO_NS(1), nullptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_timer_cancel(&canceled_timer); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_timer(&wait_set, &canceled_timer, NULL); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ - rcl_ret_t ret = rcl_guard_condition_fini(&guard_cond); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_fini(&wait_set); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ret = rcl_timer_fini(&canceled_timer); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); + ret = rcl_timer_cancel(&canceled_timer); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_timer(&wait_set, &canceled_timer, NULL); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; int64_t timeout = RCL_MS_TO_NS(10); std::chrono::steady_clock::time_point before_sc = std::chrono::steady_clock::now(); @@ -267,9 +283,6 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), canceled_timer) { // Check time int64_t diff = std::chrono::duration_cast(after_sc - before_sc).count(); EXPECT_LE(diff, RCL_MS_TO_NS(10) + TOLERANCE); - - ret = rcl_clock_fini(&clock); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } // Test rcl_wait_set_t with excess capacity works. @@ -278,6 +291,10 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), excess_capacity) { rcl_ret_t ret = rcl_wait_set_init(&wait_set, 42, 42, 42, 42, 42, 0, context_ptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_wait_set_fini(&wait_set); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); int64_t timeout = 1; ret = rcl_wait(&wait_set, timeout); @@ -358,11 +375,13 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), multi_wait_set_threade ret = rcl_guard_condition_init( &test_set.guard_condition, this->context_ptr, rcl_guard_condition_get_default_options()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + // guard_condition must live longer than this loop. Call fini from function-level exit-scope. // setup the wait set test_set.wait_set = rcl_get_zero_initialized_wait_set(); ret = rcl_wait_set_init( &test_set.wait_set, 0, 1, 0, 0, 0, 0, context_ptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + // wait_set must live longer than this loop. Call fini from function-level exit-scope. ret = rcl_wait_set_add_guard_condition(&test_set.wait_set, &test_set.guard_condition, NULL); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; test_set.thread_id = 0; @@ -421,19 +440,20 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), guard_condition) { rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 1, 0, 0, 0, 0, context_ptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_wait_set_fini(&wait_set); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); rcl_guard_condition_t guard_cond = rcl_get_zero_initialized_guard_condition(); ret = rcl_guard_condition_init( &guard_cond, this->context_ptr, rcl_guard_condition_get_default_options()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, &guard_cond, NULL); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ - rcl_ret_t ret = rcl_wait_set_fini(&wait_set); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ret = rcl_guard_condition_fini(&guard_cond); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); + ret = rcl_wait_set_add_guard_condition(&wait_set, &guard_cond, NULL); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; std::promise p; @@ -469,6 +489,10 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), add_with_index) { rcl_ret_t ret = rcl_wait_set_init( &wait_set, 0, kNumEntities, 0, 0, 0, 0, context_ptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_wait_set_fini(&wait_set); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); // Initialize to invalid value size_t guard_condition_index = 42u; @@ -479,6 +503,10 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), add_with_index) { ret = rcl_guard_condition_init( &guard_conditions[i], this->context_ptr, rcl_guard_condition_get_default_options()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + ret = rcl_guard_condition_fini(&guard_conditions[i]); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); ret = rcl_wait_set_add_guard_condition( &wait_set, &guard_conditions[i], &guard_condition_index); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;