Skip to content

Commit 2a5547a

Browse files
wjwwoodwjwwood
committed
acutally call rmw_shutdown() and address review comments
Signed-off-by: William Woodall <[email protected]>
1 parent 53e9eef commit 2a5547a

11 files changed

Lines changed: 66 additions & 16 deletions

‎rcl/src/rcl/init.c‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ extern "C"
2020
#include "rcl/init.h"
2121

2222
#include "./arguments_impl.h"
23+
#include "./common.h"
2324
#include "./context_impl.h"
2425
#include "./init_options_impl.h"
2526
#include "rcl/arguments.h"
@@ -69,11 +70,10 @@ rcl_init(
6970
context->global_arguments = rcl_get_zero_initialized_arguments();
7071

7172
// Setup impl for context.
72-
context->impl = allocator.allocate(sizeof(rcl_context_impl_t), allocator.state);
73+
// use zero_allocate so the cleanup function will not try to clean up uninitialized parts later
74+
context->impl = allocator.zero_allocate(1, sizeof(rcl_context_impl_t), allocator.state);
7375
RCL_CHECK_FOR_NULL_WITH_MSG(
7476
context->impl, "failed to allocate memory for context impl", return RCL_RET_BAD_ALLOC);
75-
// memset to 0 so the cleanup function will not try to clean up uninitialized parts later
76-
memset(context->impl, 0x0, sizeof(rcl_context_impl_t));
7777

7878
// Copy the options into the context for future reference.
7979
rcl_ret_t ret = rcl_init_options_copy(options, &(context->impl->init_options));
@@ -86,12 +86,11 @@ rcl_init(
8686
context->impl->argc = argc;
8787
context->impl->argv = NULL;
8888
if (0 != argc && argv != NULL) {
89-
context->impl->argv = (char **)allocator.allocate(sizeof(char *) * argc, allocator.state);
89+
context->impl->argv = (char **)allocator.zero_allocate(argc, sizeof(char *), allocator.state);
9090
RCL_CHECK_FOR_NULL_WITH_MSG(
9191
context->impl->argv,
9292
"failed to allocate memory for argv",
9393
fail_ret = RCL_RET_BAD_ALLOC; goto fail);
94-
memset(context->impl->argv, 0, sizeof(char *) * argc);
9594
int64_t i;
9695
for (i = 0; i < argc; ++i) {
9796
size_t argv_i_length = strlen(argv[i]);
@@ -126,6 +125,7 @@ rcl_init(
126125
goto fail;
127126
}
128127
rcutils_atomic_store((atomic_uint_least64_t *)(&context->instance_id_storage), next_instance_id);
128+
context->impl->init_options.impl->rmw_init_options.instance_id = next_instance_id;
129129

130130
// Initialize rmw_init.
131131
context->impl->rmw_context = rmw_get_zero_initialized_context();
@@ -134,7 +134,7 @@ rcl_init(
134134
&(context->impl->rmw_context));
135135
if (RMW_RET_OK != rmw_ret) {
136136
RCL_SET_ERROR_MSG(rmw_get_error_string().str);
137-
fail_ret = RCL_RET_ERROR;
137+
fail_ret = rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
138138
goto fail;
139139
}
140140

@@ -164,6 +164,12 @@ rcl_shutdown(rcl_context_t * context)
164164
// reset the instance id to 0 to indicate "invalid"
165165
rcutils_atomic_store((atomic_uint_least64_t *)(&context->instance_id_storage), 0);
166166

167+
rmw_ret_t rmw_ret = rmw_shutdown(&(context->impl->rmw_context));
168+
if (RMW_RET_OK != rmw_ret) {
169+
RCL_SET_ERROR_MSG(rmw_get_error_string().str);
170+
return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
171+
}
172+
167173
return RCL_RET_OK;
168174
}
169175

‎rcl/src/rcl/init_options.c‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ extern "C"
2323
#include "./init_options_impl.h"
2424
#include "rcl/error_handling.h"
2525
#include "rmw/error_handling.h"
26+
#include "rcutils/logging_macros.h"
2627

2728
rcl_init_options_t
2829
rcl_get_zero_initialized_init_options(void)
@@ -50,6 +51,7 @@ rcl_init_options_init(rcl_init_options_t * init_options, rcl_allocator_t allocat
5051
init_options->impl->rmw_init_options = rmw_get_zero_initialized_init_options();
5152
rmw_ret_t rmw_ret = rmw_init_options_init(&(init_options->impl->rmw_init_options), allocator);
5253
if (RMW_RET_OK != rmw_ret) {
54+
allocator.deallocate(init_options->impl, allocator.state);
5355
RCL_SET_ERROR_MSG(rmw_get_error_string().str);
5456
return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
5557
}
@@ -78,15 +80,35 @@ rcl_init_options_copy(const rcl_init_options_t * src, rcl_init_options_t * dst)
7880
// first zero-initialize rmw init options
7981
rmw_ret_t rmw_ret = rmw_init_options_fini(&(dst->impl->rmw_init_options));
8082
if (RMW_RET_OK != rmw_ret) {
81-
RCL_SET_ERROR_MSG(rmw_get_error_string().str);
83+
rmw_error_string_t error_string = rmw_get_error_string();
84+
rmw_reset_error();
85+
ret = rcl_init_options_fini(dst);
86+
if (RCL_RET_OK != ret) {
87+
RCUTILS_LOG_ERROR_NAMED(
88+
"rcl",
89+
"failed to finalize dst rcl_init_options while handling failure to "
90+
"finalize rmw_init_options, original ret '%d' and error: %s", rmw_ret, error_string.str);
91+
return ret; // error already set
92+
}
93+
RCL_SET_ERROR_MSG(error_string.str);
8294
return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
8395
}
8496
// then copy
8597
dst->impl->rmw_init_options = rmw_get_zero_initialized_init_options();
8698
rmw_ret =
8799
rmw_init_options_copy(&(src->impl->rmw_init_options), &(dst->impl->rmw_init_options));
88100
if (RMW_RET_OK != rmw_ret) {
89-
RCL_SET_ERROR_MSG(rmw_get_error_string().str);
101+
rmw_error_string_t error_string = rmw_get_error_string();
102+
rmw_reset_error();
103+
ret = rcl_init_options_fini(dst);
104+
if (RCL_RET_OK != ret) {
105+
RCUTILS_LOG_ERROR_NAMED(
106+
"rcl",
107+
"failed to finalize dst rcl_init_options while handling failure to "
108+
"copy rmw_init_options, original ret '%d' and error: %s", rmw_ret, error_string.str);
109+
return ret; // error already set
110+
}
111+
RCL_SET_ERROR_MSG(error_string.str);
90112
return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
91113
}
92114

‎rcl/test/rcl/test_count_matched.cpp‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,18 +99,24 @@ class CLASSNAME (TestCountFixture, RMW_IMPLEMENTATION) : public ::testing::Test
9999
{
100100
public:
101101
rcl_node_t * node_ptr;
102+
rcl_context_t * context_ptr;
102103
rcl_wait_set_t * wait_set_ptr;
103104
void SetUp()
104105
{
105106
rcl_ret_t ret;
106107
rcl_node_options_t node_options = rcl_node_get_default_options();
107108

108-
ret = rcl_init(0, nullptr, rcl_get_default_allocator());
109+
rcl_init_options_t init_options = rcl_get_zero_initialized_init_options();
110+
ret = rcl_init_options_init(&init_options, rcl_get_default_allocator());
111+
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
112+
this->context_ptr = new rcl_context_t;
113+
*this->context_ptr = rcl_get_zero_initialized_context();
114+
ret = rcl_init(0, nullptr, &init_options, this->context_ptr);
109115
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
110116
this->node_ptr = new rcl_node_t;
111117
*this->node_ptr = rcl_get_zero_initialized_node();
112118
const char * name = "test_count_node";
113-
ret = rcl_node_init(this->node_ptr, name, "", &node_options);
119+
ret = rcl_node_init(this->node_ptr, name, "", this->context_ptr, &node_options);
114120
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
115121

116122
this->wait_set_ptr = new rcl_wait_set_t;
@@ -130,7 +136,11 @@ class CLASSNAME (TestCountFixture, RMW_IMPLEMENTATION) : public ::testing::Test
130136
delete this->node_ptr;
131137
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
132138

133-
ret = rcl_shutdown();
139+
ret = rcl_shutdown(this->context_ptr);
140+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
141+
142+
ret = rcl_context_fini(this->context_ptr);
143+
delete this->context_ptr;
134144
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
135145
}
136146
};

‎rcl/test/rcl/test_publisher.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ class CLASSNAME (TestPublisherFixture, RMW_IMPLEMENTATION) : public ::testing::T
6565
delete this->node_ptr;
6666
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
6767
ret = rcl_shutdown(this->context_ptr);
68+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
69+
ret = rcl_context_fini(this->context_ptr);
6870
delete this->context_ptr;
6971
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
7072
}

‎rcl/test/rcl/test_service.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ class CLASSNAME (TestServiceFixture, RMW_IMPLEMENTATION) : public ::testing::Tes
6868
delete this->node_ptr;
6969
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
7070
ret = rcl_shutdown(this->context_ptr);
71+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
72+
ret = rcl_context_fini(this->context_ptr);
7173
delete this->context_ptr;
7274
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
7375
}

‎rcl/test/rcl/test_subscription.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ class CLASSNAME (TestSubscriptionFixture, RMW_IMPLEMENTATION) : public ::testing
6868
delete this->node_ptr;
6969
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
7070
ret = rcl_shutdown(this->context_ptr);
71+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
72+
ret = rcl_context_fini(this->context_ptr);
7173
delete this->context_ptr;
7274
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
7375
}

‎rcl/test/rcl/test_timer.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ class TestTimerFixture : public ::testing::Test
5757
delete this->node_ptr;
5858
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
5959
ret = rcl_shutdown(this->context_ptr);
60+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
61+
ret = rcl_context_fini(this->context_ptr);
6062
delete this->context_ptr;
6163
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
6264
}

‎rcl/test/rcl/test_wait.cpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,9 @@ class CLASSNAME (WaitSetTestFixture, RMW_IMPLEMENTATION) : public ::testing::Tes
5959

6060
void TearDown()
6161
{
62-
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({
63-
delete this->context_ptr;
64-
});
65-
ASSERT_EQ(RCL_RET_OK, rcl_shutdown(this->context_ptr)) << rcl_get_error_string().str;
66-
ASSERT_EQ(RCL_RET_OK, rcl_context_fini(this->context_ptr)) << rcl_get_error_string().str;
62+
EXPECT_EQ(RCL_RET_OK, rcl_shutdown(this->context_ptr)) << rcl_get_error_string().str;
63+
EXPECT_EQ(RCL_RET_OK, rcl_context_fini(this->context_ptr)) << rcl_get_error_string().str;
64+
delete this->context_ptr;
6765
}
6866
};
6967

‎rcl/test/test_namespace.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ class TestNamespaceFixture : public ::testing::Test
6464
delete this->node_ptr;
6565
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
6666
ret = rcl_shutdown(this->context_ptr);
67+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
68+
ret = rcl_context_fini(this->context_ptr);
6769
delete this->context_ptr;
6870
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
6971
}

‎rcl_lifecycle/test/test_default_state_machine.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ class TestDefaultStateMachine : public ::testing::Test
6969
delete this->node_ptr;
7070
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
7171
ret = rcl_shutdown(this->context_ptr);
72+
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
73+
ret = rcl_context_fini(this->context_ptr);
7274
delete this->context_ptr;
7375
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
7476
}

0 commit comments

Comments
 (0)