Skip to content

Commit 6df0003

Browse files
Acuadros95Acuadros95
authored
Fix memory release on entities creation (#280)
* Update CI * Fix memory release on entity creation Signed-off-by: acuadros95 <[email protected]> * Update node destroy on error Signed-off-by: acuadros95 <[email protected]> * Uncrustify Signed-off-by: acuadros95 <[email protected]> Signed-off-by: acuadros95 <[email protected]>
1 parent 40c1750 commit 6df0003

6 files changed

Lines changed: 42 additions & 33 deletions

File tree

‎rmw_microxrcedds_c/src/rmw_client.c‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ rmw_create_client(
3333
const char * service_name,
3434
const rmw_qos_profile_t * qos_policies)
3535
{
36+
rmw_uxrce_client_t * custom_client = NULL;
3637
rmw_client_t * rmw_client = NULL;
3738
if (!node) {
3839
RMW_UROS_TRACE_MESSAGE("node handle is null")
@@ -51,10 +52,10 @@ rmw_create_client(
5152
RMW_UROS_TRACE_MESSAGE("Not available memory node")
5253
return NULL;
5354
}
54-
rmw_uxrce_client_t * custom_client = (rmw_uxrce_client_t *)memory_node->data;
55+
custom_client = (rmw_uxrce_client_t *)memory_node->data;
5556

5657
rmw_client = &custom_client->rmw_client;
57-
rmw_client->data = NULL;
58+
rmw_client->data = custom_client;
5859
rmw_client->implementation_identifier = rmw_get_implementation_identifier();
5960
rmw_client->service_name = custom_client->service_name;
6061
if ((strlen(service_name) + 1 ) > sizeof(custom_client->service_name)) {
@@ -125,7 +126,6 @@ rmw_create_client(
125126
RMW_UXRCE_TYPE_NAME_MAX_LENGTH))
126127
{
127128
RMW_UROS_TRACE_MESSAGE("Not enough memory for service type names")
128-
put_memory(&client_memory, &custom_client->mem);
129129
goto fail;
130130
}
131131

@@ -136,7 +136,6 @@ rmw_create_client(
136136
RMW_UXRCE_TOPIC_NAME_MAX_LENGTH))
137137
{
138138
RMW_UROS_TRACE_MESSAGE("Not enough memory for service topic names")
139-
put_memory(&client_memory, &custom_client->mem);
140139
goto fail;
141140
}
142141

@@ -154,13 +153,10 @@ rmw_create_client(
154153
UXR_REPLACE | UXR_REUSE);
155154
#endif /* ifdef RMW_UXRCE_USE_XML */
156155

157-
rmw_client->data = custom_client;
158-
159156
if (!run_xrce_session(
160157
custom_node->context, custom_node->context->creation_stream, client_req,
161158
custom_node->context->creation_timeout))
162159
{
163-
put_memory(&client_memory, &custom_client->mem);
164160
goto fail;
165161
}
166162

‎rmw_microxrcedds_c/src/rmw_microxrcedds_topic.c‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,6 @@ create_topic(
5858
sizeof(rmw_uxrce_entity_naming_buffer)))
5959
{
6060
RMW_UROS_TRACE_MESSAGE("failed to generate xml request for node creation")
61-
rmw_uxrce_fini_topic_memory(custom_topic);
62-
custom_topic = NULL;
6361
goto fail;
6462
}
6563

@@ -73,15 +71,11 @@ create_topic(
7371

7472
if (!generate_topic_name(topic_name, full_topic_name, sizeof(full_topic_name))) {
7573
RMW_UROS_TRACE_MESSAGE("Error creating topic name");
76-
rmw_uxrce_fini_topic_memory(custom_topic);
77-
custom_topic = NULL;
7874
goto fail;
7975
}
8076

8177
if (!generate_type_name(message_type_support_callbacks, type_name, sizeof(type_name))) {
8278
RMW_UROS_TRACE_MESSAGE("Error creating type name");
83-
rmw_uxrce_fini_topic_memory(custom_topic);
84-
custom_topic = NULL;
8579
goto fail;
8680
}
8781

@@ -99,12 +93,17 @@ create_topic(
9993
custom_node->context, custom_node->context->creation_stream, topic_req,
10094
custom_node->context->creation_timeout))
10195
{
102-
rmw_uxrce_fini_topic_memory(custom_topic);
103-
custom_topic = NULL;
10496
goto fail;
10597
}
10698

99+
return custom_topic;
100+
107101
fail:
102+
if (custom_topic != NULL) {
103+
rmw_uxrce_fini_topic_memory(custom_topic);
104+
}
105+
106+
custom_topic = NULL;
108107
return custom_topic;
109108
}
110109

‎rmw_microxrcedds_c/src/rmw_node.c‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,11 @@ rmw_ret_t rmw_destroy_node(
169169
item = item->next;
170170
if (custom_publisher->owner_node == custom_node) {
171171
ret = rmw_destroy_publisher(node, &custom_publisher->rmw_publisher);
172+
173+
// We should not early return on a RMW_RET_TIMEOUT, as it may be an expected output
174+
if (RMW_RET_ERROR == ret) {
175+
return ret;
176+
}
172177
}
173178
}
174179

@@ -178,6 +183,10 @@ rmw_ret_t rmw_destroy_node(
178183
item = item->next;
179184
if (custom_subscription->owner_node == custom_node) {
180185
ret = rmw_destroy_subscription(node, &custom_subscription->rmw_subscription);
186+
187+
if (RMW_RET_ERROR == ret) {
188+
return ret;
189+
}
181190
}
182191
}
183192

@@ -187,6 +196,10 @@ rmw_ret_t rmw_destroy_node(
187196
item = item->next;
188197
if (custom_service->owner_node == custom_node) {
189198
ret = rmw_destroy_service(node, &custom_service->rmw_service);
199+
200+
if (RMW_RET_ERROR == ret) {
201+
return ret;
202+
}
190203
}
191204
}
192205

@@ -196,6 +209,10 @@ rmw_ret_t rmw_destroy_node(
196209
item = item->next;
197210
if (custom_client->owner_node == custom_node) {
198211
ret = rmw_destroy_client(node, &custom_client->rmw_client);
212+
213+
if (RMW_RET_ERROR == ret) {
214+
return ret;
215+
}
199216
}
200217
}
201218

‎rmw_microxrcedds_c/src/rmw_publisher.c‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ rmw_create_publisher(
6767
{
6868
(void)publisher_options;
6969

70+
rmw_uxrce_publisher_t * custom_publisher = NULL;
7071
rmw_publisher_t * rmw_publisher = NULL;
7172
if (!node) {
7273
RMW_UROS_TRACE_MESSAGE("node handle is null")
@@ -85,10 +86,10 @@ rmw_create_publisher(
8586
RMW_UROS_TRACE_MESSAGE("Not available memory node")
8687
return NULL;
8788
}
88-
rmw_uxrce_publisher_t * custom_publisher = (rmw_uxrce_publisher_t *)memory_node->data;
89+
custom_publisher = (rmw_uxrce_publisher_t *)memory_node->data;
8990

9091
rmw_publisher = &custom_publisher->rmw_publisher;
91-
rmw_publisher->data = NULL;
92+
rmw_publisher->data = custom_publisher;
9293
rmw_publisher->implementation_identifier = rmw_get_implementation_identifier();
9394
rmw_publisher->topic_name = custom_publisher->topic_name;
9495

@@ -171,12 +172,9 @@ rmw_create_publisher(
171172
custom_node->context, custom_node->context->creation_stream, publisher_req,
172173
custom_node->context->creation_timeout))
173174
{
174-
put_memory(&publisher_memory, &custom_publisher->mem);
175175
goto fail;
176176
}
177177

178-
rmw_publisher->data = custom_publisher;
179-
180178
// Create datawriter
181179
custom_publisher->datawriter_id = uxr_object_id(
182180
custom_node->context->id_datawriter++,
@@ -212,13 +210,16 @@ rmw_create_publisher(
212210
custom_node->context, custom_node->context->creation_stream, datawriter_req,
213211
custom_node->context->creation_timeout))
214212
{
215-
put_memory(&publisher_memory, &custom_publisher->mem);
216213
goto fail;
217214
}
218215
}
219216

220217
return rmw_publisher;
221218
fail:
219+
if (custom_publisher != NULL && custom_publisher->topic != NULL) {
220+
rmw_uxrce_fini_topic_memory(custom_publisher->topic);
221+
}
222+
222223
rmw_uxrce_fini_publisher_memory(rmw_publisher);
223224
rmw_publisher = NULL;
224225
return rmw_publisher;

‎rmw_microxrcedds_c/src/rmw_service.c‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ rmw_create_service(
5454
rmw_uxrce_service_t * custom_service = (rmw_uxrce_service_t *)memory_node->data;
5555

5656
rmw_service = &custom_service->rmw_service;
57-
rmw_service->data = NULL;
57+
rmw_service->data = custom_service;
5858
rmw_service->implementation_identifier = rmw_get_implementation_identifier();
5959
rmw_service->service_name = custom_service->service_name;
6060
if ((strlen(service_name) + 1 ) > sizeof(custom_service->service_name)) {
@@ -123,7 +123,6 @@ rmw_create_service(
123123
RMW_UXRCE_TYPE_NAME_MAX_LENGTH))
124124
{
125125
RMW_UROS_TRACE_MESSAGE("Not enough memory for service type names")
126-
put_memory(&service_memory, &custom_service->mem);
127126
goto fail;
128127
}
129128

@@ -134,7 +133,6 @@ rmw_create_service(
134133
RMW_UXRCE_TOPIC_NAME_MAX_LENGTH))
135134
{
136135
RMW_UROS_TRACE_MESSAGE("Not enough memory for service topic names")
137-
put_memory(&service_memory, &custom_service->mem);
138136
goto fail;
139137
}
140138

@@ -152,14 +150,11 @@ rmw_create_service(
152150
UXR_REPLACE | UXR_REUSE);
153151
#endif /* ifdef RMW_UXRCE_USE_XML */
154152

155-
rmw_service->data = custom_service;
156-
157153
if (!run_xrce_session(
158154
custom_node->context, custom_node->context->creation_stream, service_req,
159155
custom_node->context->creation_timeout))
160156
{
161157
RMW_UROS_TRACE_MESSAGE("Issues creating Micro XRCE-DDS entities")
162-
put_memory(&service_memory, &custom_service->mem);
163158
goto fail;
164159
}
165160

‎rmw_microxrcedds_c/src/rmw_subscription.c‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ rmw_create_subscription(
6565
{
6666
(void)subscription_options;
6767

68+
rmw_uxrce_subscription_t * custom_subscription = NULL;
6869
rmw_subscription_t * rmw_subscription = NULL;
6970
if (!node) {
7071
RMW_UROS_TRACE_MESSAGE("node handle is null")
@@ -85,10 +86,10 @@ rmw_create_subscription(
8586
RMW_UROS_TRACE_MESSAGE("Not available memory node")
8687
return NULL;
8788
}
88-
rmw_uxrce_subscription_t * custom_subscription = (rmw_uxrce_subscription_t *)memory_node->data;
89+
custom_subscription = (rmw_uxrce_subscription_t *)memory_node->data;
8990

9091
rmw_subscription = &custom_subscription->rmw_subscription;
91-
rmw_subscription->data = NULL;
92+
rmw_subscription->data = custom_subscription;
9293
rmw_subscription->implementation_identifier = rmw_get_implementation_identifier();
9394
rmw_subscription->topic_name = custom_subscription->topic_name;
9495
if ((strlen(topic_name) + 1 ) > sizeof(custom_subscription->topic_name)) {
@@ -159,7 +160,6 @@ rmw_create_subscription(
159160
custom_node->context, custom_node->context->creation_stream, subscriber_req,
160161
custom_node->context->creation_timeout))
161162
{
162-
put_memory(&subscription_memory, &custom_subscription->mem);
163163
goto fail;
164164
}
165165

@@ -198,12 +198,9 @@ rmw_create_subscription(
198198
custom_node->context->creation_timeout))
199199
{
200200
RMW_UROS_TRACE_MESSAGE("Issues creating Micro XRCE-DDS entities")
201-
put_memory(&subscription_memory, &custom_subscription->mem);
202201
goto fail;
203202
}
204203

205-
rmw_subscription->data = custom_subscription;
206-
207204
uxrDeliveryControl delivery_control;
208205
delivery_control.max_samples = UXR_MAX_SAMPLES_UNLIMITED;
209206
delivery_control.min_pace_period = 0;
@@ -223,6 +220,10 @@ rmw_create_subscription(
223220
return rmw_subscription;
224221

225222
fail:
223+
if (custom_subscription != NULL && custom_subscription->topic != NULL) {
224+
rmw_uxrce_fini_topic_memory(custom_subscription->topic);
225+
}
226+
226227
rmw_uxrce_fini_subscription_memory(rmw_subscription);
227228
rmw_subscription = NULL;
228229
return rmw_subscription;

0 commit comments

Comments
 (0)