Skip to content

Commit 5bfccd7

Browse files
author
Minggang Wang
committed
Fix the memory leak cases
This patch mainly fixed two memory leak circumstances: 1. When calling rcl_take_response, a pointer of type of rmw_request_id_t is passed, but never gets deallocated. We should free the memory immediately. 2. After allocating a pointer of rmw_qos_profile_t, we have to deallocate it. So a unique_ptr will take the ownship of it. These problems above should not be the root cause of issue#175.
1 parent 4c7f573 commit 5bfccd7

1 file changed

Lines changed: 31 additions & 16 deletions

File tree

src/rcl_bindings.cpp

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
#include <rmw/validate_namespace.h>
2626
#include <rmw/validate_node_name.h>
2727
#include <rosidl_generator_c/string_functions.h>
28+
29+
#include <memory>
2830
#include <string>
2931

3032
#include "handle_manager.hpp"
@@ -35,7 +37,7 @@
3537

3638
namespace rclnodejs {
3739

38-
const rmw_qos_profile_t* GetQoSProfile(v8::Local<v8::Value> qos);
40+
rmw_qos_profile_t* GetQoSProfile(v8::Local<v8::Value> qos);
3941

4042
NAN_METHOD(Init) {
4143
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK,
@@ -188,7 +190,9 @@ NAN_METHOD(CreateSubscription) {
188190

189191
rcl_subscription_options_t subscription_ops =
190192
rcl_subscription_get_default_options();
191-
const rmw_qos_profile_t* qos_profile = GetQoSProfile(info[5]);
193+
auto qos_profile = std::make_unique<rmw_qos_profile_t>();
194+
qos_profile.reset(GetQoSProfile(info[5]));
195+
192196
if (qos_profile) {
193197
subscription_ops.qos = *qos_profile;
194198
}
@@ -229,7 +233,9 @@ NAN_METHOD(CreatePublisher) {
229233

230234
// Using default options
231235
rcl_publisher_options_t publisher_ops = rcl_publisher_get_default_options();
232-
const rmw_qos_profile_t* qos_profile = GetQoSProfile(info[5]);
236+
auto qos_profile = std::make_unique<rmw_qos_profile_t>();
237+
qos_profile.reset(GetQoSProfile(info[5]));
238+
233239
if (qos_profile) {
234240
publisher_ops.qos = *qos_profile;
235241
}
@@ -275,7 +281,9 @@ NAN_METHOD(CreateClient) {
275281
reinterpret_cast<rcl_client_t*>(malloc(sizeof(rcl_client_t)));
276282
*client = rcl_get_zero_initialized_client();
277283
rcl_client_options_t client_ops = rcl_client_get_default_options();
278-
const rmw_qos_profile_t* qos_profile = GetQoSProfile(info[4]);
284+
auto qos_profile = std::make_unique<rmw_qos_profile_t>();
285+
qos_profile.reset(GetQoSProfile(info[4]));
286+
279287
if (qos_profile) {
280288
client_ops.qos = *qos_profile;
281289
}
@@ -310,12 +318,13 @@ NAN_METHOD(RclTakeResponse) {
310318
RclHandle::Unwrap<RclHandle>(info[0]->ToObject())->ptr());
311319
int64_t sequence_number = info[1]->IntegerValue();
312320

313-
rmw_request_id_t * header =
321+
rmw_request_id_t* header =
314322
reinterpret_cast<rmw_request_id_t*>(malloc(sizeof(rmw_request_id_t)));
315323
header->sequence_number = sequence_number;
316324

317325
void* taken_response = node::Buffer::Data(info[2]->ToObject());
318326
rcl_ret_t ret = rcl_take_response(client, header, taken_response);
327+
free(header);
319328

320329
if (ret == RCL_RET_OK) {
321330
info.GetReturnValue().Set(Nan::True());
@@ -339,7 +348,9 @@ NAN_METHOD(CreateService) {
339348
reinterpret_cast<rcl_service_t*>(malloc(sizeof(rcl_service_t)));
340349
*service = rcl_get_zero_initialized_service();
341350
rcl_service_options_t service_ops = rcl_service_get_default_options();
342-
const rmw_qos_profile_t* qos_profile = GetQoSProfile(info[4]);
351+
auto qos_profile = std::make_unique<rmw_qos_profile_t>();
352+
qos_profile.reset(GetQoSProfile(info[4]));
353+
343354
if (qos_profile) {
344355
service_ops.qos = *qos_profile;
345356
}
@@ -406,7 +417,7 @@ NAN_METHOD(ValidateFullTopicName) {
406417
info.GetReturnValue().Set(Nan::Null());
407418
return;
408419
}
409-
const char * validation_message =
420+
const char* validation_message =
410421
rmw_full_topic_name_validation_result_string(validation_result);
411422
THROW_ERROR_IF_EQUAL(nullptr, validation_message,
412423
"Unable to get validation error message");
@@ -440,7 +451,7 @@ NAN_METHOD(ValidateNodeName) {
440451
info.GetReturnValue().Set(Nan::Null());
441452
return;
442453
}
443-
const char * validation_message =
454+
const char* validation_message =
444455
rmw_node_name_validation_result_string(validation_result);
445456
THROW_ERROR_IF_EQUAL(nullptr, validation_message,
446457
"Unable to get validation error message");
@@ -474,7 +485,7 @@ NAN_METHOD(ValidateTopicName) {
474485
info.GetReturnValue().Set(Nan::Null());
475486
return;
476487
}
477-
const char * validation_message =
488+
const char* validation_message =
478489
rcl_topic_name_validation_result_string(validation_result);
479490
THROW_ERROR_IF_EQUAL(nullptr, validation_message,
480491
"Unable to get validation error message");
@@ -508,7 +519,7 @@ NAN_METHOD(ValidateNamespace) {
508519
info.GetReturnValue().Set(Nan::Null());
509520
return;
510521
}
511-
const char * validation_message =
522+
const char* validation_message =
512523
rmw_namespace_validation_result_string(validation_result);
513524
THROW_ERROR_IF_EQUAL(nullptr, validation_message,
514525
"Unable to get validation error message");
@@ -595,7 +606,7 @@ NAN_METHOD(ExpandTopicName) {
595606
NAN_METHOD(GetNodeName) {
596607
RclHandle* node_handle = RclHandle::Unwrap<RclHandle>(info[0]->ToObject());
597608
rcl_node_t* node = reinterpret_cast<rcl_node_t*>(node_handle->ptr());
598-
const char * node_name = rcl_node_get_name(node);
609+
const char* node_name = rcl_node_get_name(node);
599610
if (!node_name) {
600611
info.GetReturnValue().Set(Nan::Undefined());
601612
} else {
@@ -606,7 +617,7 @@ NAN_METHOD(GetNodeName) {
606617
NAN_METHOD(GetNamespace) {
607618
RclHandle* node_handle = RclHandle::Unwrap<RclHandle>(info[0]->ToObject());
608619
rcl_node_t* node = reinterpret_cast<rcl_node_t*>(node_handle->ptr());
609-
const char * node_namespace = rcl_node_get_namespace(node);
620+
const char* node_namespace = rcl_node_get_namespace(node);
610621
if (!node_namespace) {
611622
info.GetReturnValue().Set(Nan::Undefined());
612623
} else {
@@ -638,7 +649,7 @@ const rmw_qos_profile_t* GetQoSProfileFromString(
638649
}
639650

640651
const rmw_qos_profile_t* GetQosProfileFromObject(v8::Local<v8::Object> object) {
641-
rmw_qos_profile_t * qos_profile = reinterpret_cast<rmw_qos_profile_t*>(
652+
rmw_qos_profile_t* qos_profile = reinterpret_cast<rmw_qos_profile_t*>(
642653
malloc(sizeof(rmw_qos_profile_t)));
643654
qos_profile->history = static_cast<rmw_qos_history_policy_t>(
644655
object->Get(Nan::New("history").ToLocalChecked())->Uint32Value());
@@ -655,15 +666,19 @@ const rmw_qos_profile_t* GetQosProfileFromObject(v8::Local<v8::Object> object) {
655666
return qos_profile;
656667
}
657668

658-
const rmw_qos_profile_t* GetQoSProfile(v8::Local<v8::Value> qos) {
669+
rmw_qos_profile_t* GetQoSProfile(v8::Local<v8::Value> qos) {
670+
rmw_qos_profile_t* qos_profile = reinterpret_cast<rmw_qos_profile_t*>(
671+
malloc(sizeof(rmw_qos_profile_t)));
672+
659673
if (qos->IsString()) {
660-
return GetQoSProfileFromString(
674+
*qos_profile = *GetQoSProfileFromString(
661675
std::string(*Nan::Utf8String(qos->ToString())));
662676
} else if (qos->IsObject()) {
663-
return GetQosProfileFromObject(qos->ToObject());
677+
*qos_profile = *GetQosProfileFromObject(qos->ToObject());
664678
} else {
665679
return nullptr;
666680
}
681+
return qos_profile;
667682
}
668683

669684
NAN_METHOD(Shutdown) {

0 commit comments

Comments
 (0)