diff --git a/rcl/src/rcl/wait.c b/rcl/src/rcl/wait.c index 79556df9f4..3e8d00b7c1 100644 --- a/rcl/src/rcl/wait.c +++ b/rcl/src/rcl/wait.c @@ -86,13 +86,13 @@ rcl_wait_set_is_valid(const rcl_wait_set_t * wait_set) } static void -__wait_set_clean_up(rcl_wait_set_t * wait_set, rcl_allocator_t allocator) +__wait_set_clean_up(rcl_wait_set_t * wait_set) { rcl_ret_t ret = rcl_wait_set_resize(wait_set, 0, 0, 0, 0, 0, 0); (void)ret; // NO LINT assert(RCL_RET_OK == ret); // Defensive, shouldn't fail with size 0. if (wait_set->impl) { - allocator.deallocate(wait_set->impl, allocator.state); + wait_set->impl->allocator.deallocate(wait_set->impl, wait_set->impl->allocator.state); wait_set->impl = NULL; } } @@ -146,6 +146,10 @@ rcl_wait_set_init( wait_set->impl->rmw_services.service_count = 0; wait_set->impl->rmw_events.events = NULL; wait_set->impl->rmw_events.event_count = 0; + // Set context. + wait_set->impl->context = context; + // Set allocator. + wait_set->impl->allocator = allocator; size_t num_conditions = (2 * number_of_subscriptions) + @@ -159,10 +163,6 @@ rcl_wait_set_init( goto fail; } - // Set context. - wait_set->impl->context = context; - // Set allocator. - wait_set->impl->allocator = allocator; // Initialize subscription space. rcl_ret_t ret = rcl_wait_set_resize( wait_set, number_of_subscriptions, number_of_guard_conditions, number_of_timers, @@ -179,7 +179,7 @@ rcl_wait_set_init( fail_ret = RCL_RET_WAIT_SET_INVALID; } } - __wait_set_clean_up(wait_set, allocator); + __wait_set_clean_up(wait_set); return fail_ret; } @@ -195,7 +195,7 @@ rcl_wait_set_fini(rcl_wait_set_t * wait_set) RCL_SET_ERROR_MSG(rmw_get_error_string().str); result = RCL_RET_WAIT_SET_INVALID; } - __wait_set_clean_up(wait_set, wait_set->impl->allocator); + __wait_set_clean_up(wait_set); } return result; } @@ -300,6 +300,7 @@ rcl_wait_set_get_allocator(const rcl_wait_set_t * wait_set, rcl_allocator_t * al wait_set->impl->RMWStorage, sizeof(void *) * Type ## s_size, allocator.state); \ if (!wait_set->impl->RMWStorage) { \ allocator.deallocate((void *)wait_set->Type ## s, allocator.state); \ + wait_set->Type ## s = NULL; \ wait_set->size_of_ ## Type ## s = 0; \ RCL_SET_ERROR_MSG("allocating memory failed"); \ return RCL_RET_BAD_ALLOC; \ @@ -381,6 +382,7 @@ rcl_wait_set_resize( { RCL_CHECK_ARGUMENT_FOR_NULL(wait_set, RCL_RET_INVALID_ARGUMENT); RCL_CHECK_ARGUMENT_FOR_NULL(wait_set->impl, RCL_RET_WAIT_SET_INVALID); + SET_RESIZE( subscription, SET_RESIZE_RMW_DEALLOC( diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index 17d9c0ac2e..52423d0edc 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -264,7 +264,7 @@ function(test_target_function) SRCS rcl/test_wait.cpp ENV ${rmw_implementation_env_var} APPEND_LIBRARY_DIRS ${extra_lib_dirs} - LIBRARIES ${PROJECT_NAME} + LIBRARIES ${PROJECT_NAME} mimick AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" ) diff --git a/rcl/test/rcl/test_wait.cpp b/rcl/test/rcl/test_wait.cpp index 82244406f3..c903b6f91e 100644 --- a/rcl/test/rcl/test_wait.cpp +++ b/rcl/test/rcl/test_wait.cpp @@ -30,6 +30,7 @@ #include "rcutils/logging_macros.h" #include "./allocator_testing_utils.h" +#include "../mocking_utils/patch.hpp" #ifdef RMW_IMPLEMENTATION # define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX @@ -684,3 +685,68 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_get_allocator ret = rcl_wait_set_fini(&wait_set); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } + +// Test wait set init failure cases using mocks +TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_failed_init) { + rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); + // nullptr failures + rcl_ret_t ret = + rcl_wait_set_init(nullptr, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator()); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); + EXPECT_TRUE(rcl_error_is_set()); + rcl_reset_error(); + + ret = + rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, nullptr, rcl_get_default_allocator()); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); + EXPECT_TRUE(rcl_error_is_set()); + rcl_reset_error(); + + { + // Mock rmw implementation to fail init + auto mock = mocking_utils::patch_and_return( + "lib:rcl", rmw_create_wait_set, nullptr); + ret = + rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator()); + EXPECT_EQ(RCL_RET_WAIT_SET_INVALID, ret); + EXPECT_TRUE(rcl_error_is_set()); + rcl_reset_error(); + } +} + +// Test failure init when using a bad allocators +TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_failed_init_bomb_alloc) { + rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); + rcl_ret_t ret = RCL_RET_OK; + + // Pass bomb allocator to make resize function fail + rcl_allocator_t bomb_alloc = get_time_bombed_allocator(); + for (int i = 0; i < 10; i++) { + set_time_bombed_allocator_count(bomb_alloc, i); + ret = + rcl_wait_set_init(&wait_set, 1, 0, 0, 1, 1, 0, context_ptr, bomb_alloc); + if (RCL_RET_OK == ret) { + EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set)); + break; + } else { + EXPECT_EQ(RCL_RET_BAD_ALLOC, ret); + rcl_reset_error(); + } + } +} + +// Test wait set fini failure cases using mocks +TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_failed_fini) { + rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); + rcl_ret_t ret = + rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator()); + ASSERT_EQ(RCL_RET_OK, ret); + { + // Mock rmw implementation to fail fini + auto mock = mocking_utils::inject_on_return( + "lib:rcl", rmw_destroy_wait_set, RMW_RET_ERROR); + EXPECT_EQ(RCL_RET_WAIT_SET_INVALID, rcl_wait_set_fini(&wait_set)); + EXPECT_TRUE(rcl_error_is_set()); + rcl_reset_error(); + } +}