Skip to content

Commit 2b75642

Browse files
author
Minggang Wang
committed
Use specified deleter for RclHandle
This patch will enable RclHandle to use a specified deleter to free the resources, which means the pointer passed when contructing the RclHandle object will not be freed by default. After this patch, caller should be responsible to manage the memory. Fix #728
1 parent 085d5c0 commit 2b75642

File tree

7 files changed

+133
-93
lines changed

7 files changed

+133
-93
lines changed

‎src/executor.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -180,11 +180,11 @@ RclResult Executor::WaitForReadyCallbacks(rcl_wait_set_t* wait_set,
180180
return RclResult(get_entity_ret, error_message);
181181
}
182182

183-
rcl_ret_t resize_ret = rcl_wait_set_resize(
184-
wait_set, num_subscriptions, num_guard_conditions, num_timers,
185-
num_clients, num_services,
186-
// TODO(minggang): support events.
187-
0u);
183+
rcl_ret_t resize_ret =
184+
rcl_wait_set_resize(wait_set, num_subscriptions, num_guard_conditions,
185+
num_timers, num_clients, num_services,
186+
// TODO(minggang): support events.
187+
0u);
188188
if (resize_ret != RCL_RET_OK) {
189189
std::string error_message = std::string("Failed to resize: ") +
190190
std::string(rcl_get_error_string().str);

‎src/macros.hpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
if (lhs op rhs) { \
2323
Nan::ThrowError(message); \
2424
rcl_reset_error(); \
25-
info.GetReturnValue().Set(Nan::Undefined()); \
2625
return; \
2726
} \
2827
}

‎src/rcl_action_bindings.cpp‎

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,13 @@ NAN_METHOD(ActionCreateClient) {
8080
rcl_action_client_init(action_client, node, ts, action_name.c_str(),
8181
&action_client_ops),
8282
RCL_RET_OK, rcl_get_error_string().str);
83-
auto js_obj = RclHandle::NewInstance(
84-
action_client, node_handle, [action_client, node] {
85-
return rcl_action_client_fini(action_client, node);
83+
auto js_obj =
84+
RclHandle::NewInstance(action_client, node_handle, [node](void* ptr) {
85+
rcl_action_client_t* action_client =
86+
reinterpret_cast<rcl_action_client_t*>(ptr);
87+
rcl_ret_t ret = rcl_action_client_fini(action_client, node);
88+
free(ptr);
89+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
8690
});
8791

8892
info.GetReturnValue().Set(js_obj);
@@ -147,9 +151,13 @@ NAN_METHOD(ActionCreateServer) {
147151
rcl_action_server_init(action_server, node, clock, ts,
148152
action_name.c_str(), &action_server_ops),
149153
RCL_RET_OK, rcl_get_error_string().str);
150-
auto js_obj = RclHandle::NewInstance(
151-
action_server, node_handle, [action_server, node] {
152-
return rcl_action_server_fini(action_server, node);
154+
auto js_obj =
155+
RclHandle::NewInstance(action_server, node_handle, [node](void* ptr) {
156+
rcl_action_server_t* action_server =
157+
reinterpret_cast<rcl_action_server_t*>(ptr);
158+
rcl_ret_t ret = rcl_action_server_fini(action_server, node);
159+
free(ptr);
160+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
153161
});
154162

155163
info.GetReturnValue().Set(js_obj);
@@ -208,7 +216,8 @@ NAN_METHOD(ActionTakeGoalRequest) {
208216
rcl_ret_t ret =
209217
rcl_action_take_goal_request(action_server, header, taken_request);
210218
if (ret != RCL_RET_ACTION_SERVER_TAKE_FAILED) {
211-
auto js_obj = RclHandle::NewInstance(header);
219+
auto js_obj =
220+
RclHandle::NewInstance(header, nullptr, [](void* ptr) { free(ptr); });
212221
info.GetReturnValue().Set(js_obj);
213222
return;
214223
}
@@ -296,7 +305,8 @@ NAN_METHOD(ActionTakeCancelRequest) {
296305
rcl_ret_t ret =
297306
rcl_action_take_cancel_request(action_server, header, taken_request);
298307
if (ret != RCL_RET_ACTION_SERVER_TAKE_FAILED) {
299-
auto js_obj = RclHandle::NewInstance(header);
308+
auto js_obj =
309+
RclHandle::NewInstance(header, nullptr, [](void* ptr) { free(ptr); });
300310
info.GetReturnValue().Set(js_obj);
301311
return;
302312
}
@@ -386,7 +396,8 @@ NAN_METHOD(ActionTakeResultRequest) {
386396
rcl_ret_t ret =
387397
rcl_action_take_result_request(action_server, header, taken_request);
388398
if (ret != RCL_RET_ACTION_SERVER_TAKE_FAILED) {
389-
auto js_obj = RclHandle::NewInstance(header);
399+
auto js_obj =
400+
RclHandle::NewInstance(header, nullptr, [](void* ptr) { free(ptr); });
390401
info.GetReturnValue().Set(js_obj);
391402
return;
392403
}
@@ -464,8 +475,12 @@ NAN_METHOD(ActionAcceptNewGoal) {
464475
return;
465476
}
466477

467-
auto js_obj = RclHandle::NewInstance(goal_handle, nullptr, [goal_handle] {
468-
return rcl_action_goal_handle_fini(goal_handle);
478+
auto js_obj = RclHandle::NewInstance(goal_handle, nullptr, [](void* ptr) {
479+
rcl_action_goal_handle_t* goal_handle =
480+
reinterpret_cast<rcl_action_goal_handle_t*>(ptr);
481+
rcl_ret_t ret = rcl_action_goal_handle_fini(goal_handle);
482+
free(ptr);
483+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
469484
});
470485

471486
info.GetReturnValue().Set(js_obj);
@@ -646,9 +661,13 @@ NAN_METHOD(ActionProcessCancelRequest) {
646661
}
647662

648663
*response = cancel_response_ptr->msg;
649-
auto js_obj = RclHandle::NewInstance(
650-
cancel_response_ptr, nullptr, [cancel_response_ptr] {
651-
return rcl_action_cancel_response_fini(cancel_response_ptr);
664+
auto js_obj =
665+
RclHandle::NewInstance(cancel_response_ptr, nullptr, [](void* ptr) {
666+
rcl_action_cancel_response_t* cancel_response_ptr =
667+
reinterpret_cast<rcl_action_cancel_response_t*>(ptr);
668+
rcl_ret_t ret = rcl_action_cancel_response_fini(cancel_response_ptr);
669+
free(ptr);
670+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
652671
});
653672
info.GetReturnValue().Set(js_obj);
654673
}

‎src/rcl_bindings.cpp‎

Lines changed: 65 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,12 @@ NAN_METHOD(CreateNode) {
258258
name_space.c_str(), context, &options),
259259
rcl_get_error_string().str);
260260

261-
auto handle = RclHandle::NewInstance(node, nullptr,
262-
[node] { return rcl_node_fini(node); });
261+
auto handle = RclHandle::NewInstance(node, nullptr, [](void* ptr) {
262+
rcl_node_t* node = reinterpret_cast<rcl_node_t*>(ptr);
263+
rcl_ret_t ret = rcl_node_fini(node);
264+
free(ptr);
265+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
266+
});
263267
info.GetReturnValue().Set(handle);
264268
}
265269

@@ -280,8 +284,12 @@ NAN_METHOD(CreateGuardCondition) {
280284
rcl_guard_condition_init(gc, context, gc_options),
281285
rcl_get_error_string().str);
282286

283-
auto handle = RclHandle::NewInstance(
284-
gc, nullptr, [gc] { return rcl_guard_condition_fini(gc); });
287+
auto handle = RclHandle::NewInstance(gc, nullptr, [](void* ptr) {
288+
rcl_guard_condition_t* gc = reinterpret_cast<rcl_guard_condition_t*>(ptr);
289+
rcl_ret_t ret = rcl_guard_condition_fini(gc);
290+
free(ptr);
291+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
292+
});
285293
info.GetReturnValue().Set(handle);
286294
}
287295

@@ -315,8 +323,12 @@ NAN_METHOD(CreateTimer) {
315323
rcl_get_default_allocator()),
316324
rcl_get_error_string().str);
317325

318-
auto js_obj = RclHandle::NewInstance(
319-
timer, clock_handle, [timer] { return rcl_timer_fini(timer); });
326+
auto js_obj = RclHandle::NewInstance(timer, clock_handle, [](void* ptr) {
327+
rcl_timer_t* timer = reinterpret_cast<rcl_timer_t*>(ptr);
328+
rcl_ret_t ret = rcl_timer_fini(timer);
329+
free(ptr);
330+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
331+
});
320332
info.GetReturnValue().Set(js_obj);
321333
}
322334

@@ -411,7 +423,8 @@ NAN_METHOD(CreateTimePoint) {
411423
time_point->nanoseconds = std::stoll(str);
412424
time_point->clock_type = static_cast<rcl_clock_type_t>(clock_type);
413425

414-
auto js_obj = RclHandle::NewInstance(time_point, nullptr, nullptr);
426+
auto js_obj =
427+
RclHandle::NewInstance(time_point, nullptr, [](void* ptr) { free(ptr); });
415428
info.GetReturnValue().Set(js_obj);
416429
}
417430

@@ -431,7 +444,8 @@ NAN_METHOD(CreateDuration) {
431444
reinterpret_cast<rcl_duration_t*>(malloc(sizeof(rcl_duration_t)));
432445
duration->nanoseconds = std::stoll(str);
433446

434-
auto js_obj = RclHandle::NewInstance(duration, nullptr, nullptr);
447+
auto js_obj =
448+
RclHandle::NewInstance(duration, nullptr, [](void* ptr) { free(ptr); });
435449
info.GetReturnValue().Set(js_obj);
436450
}
437451

@@ -500,8 +514,13 @@ NAN_METHOD(CreateClock) {
500514
rcl_clock_init(clock_type, clock, &allocator),
501515
rcl_get_error_string().str);
502516

503-
info.GetReturnValue().Set(RclHandle::NewInstance(
504-
clock, nullptr, [clock]() { return rcl_clock_fini(clock); }));
517+
info.GetReturnValue().Set(
518+
RclHandle::NewInstance(clock, nullptr, [](void* ptr) {
519+
rcl_clock_t* clock = reinterpret_cast<rcl_clock_t*>(ptr);
520+
rcl_ret_t ret = rcl_clock_fini(clock);
521+
free(ptr);
522+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
523+
}));
505524
}
506525

507526
static void ReturnJSTimeObj(
@@ -654,8 +673,12 @@ NAN_METHOD(CreateSubscription) {
654673
rcl_get_error_string().str);
655674

656675
auto js_obj =
657-
RclHandle::NewInstance(subscription, node_handle, [subscription, node] {
658-
return rcl_subscription_fini(subscription, node);
676+
RclHandle::NewInstance(subscription, node_handle, [node](void* ptr) {
677+
rcl_subscription_t* subscription =
678+
reinterpret_cast<rcl_subscription_t*>(ptr);
679+
rcl_ret_t ret = rcl_subscription_fini(subscription, node);
680+
free(ptr);
681+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
659682
});
660683
info.GetReturnValue().Set(js_obj);
661684
} else {
@@ -702,9 +725,13 @@ NAN_METHOD(CreatePublisher) {
702725
RCL_RET_OK, rcl_get_error_string().str);
703726

704727
// Wrap the handle into JS object
705-
auto js_obj = RclHandle::NewInstance(
706-
publisher, node_handle,
707-
[publisher, node]() { return rcl_publisher_fini(publisher, node); });
728+
auto js_obj =
729+
RclHandle::NewInstance(publisher, node_handle, [node](void* ptr) {
730+
rcl_publisher_t* publisher = reinterpret_cast<rcl_publisher_t*>(ptr);
731+
rcl_ret_t ret = rcl_publisher_fini(publisher, node);
732+
free(ptr);
733+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
734+
});
708735

709736
// Everything is done
710737
info.GetReturnValue().Set(js_obj);
@@ -767,9 +794,13 @@ NAN_METHOD(CreateClient) {
767794
rcl_client_init(client, node, ts, service_name.c_str(), &client_ops),
768795
RCL_RET_OK, rcl_get_error_string().str);
769796

770-
auto js_obj = RclHandle::NewInstance(client, node_handle, [client, node] {
771-
return rcl_client_fini(client, node);
772-
});
797+
auto js_obj =
798+
RclHandle::NewInstance(client, node_handle, [node](void* ptr) {
799+
rcl_client_t* client = reinterpret_cast<rcl_client_t*>(ptr);
800+
rcl_ret_t ret = rcl_client_fini(client, node);
801+
free(ptr);
802+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
803+
});
773804

774805
info.GetReturnValue().Set(js_obj);
775806
} else {
@@ -846,9 +877,13 @@ NAN_METHOD(CreateService) {
846877
THROW_ERROR_IF_NOT_EQUAL(
847878
rcl_service_init(service, node, ts, service_name.c_str(), &service_ops),
848879
RCL_RET_OK, rcl_get_error_string().str);
849-
auto js_obj = RclHandle::NewInstance(service, node_handle, [service, node] {
850-
return rcl_service_fini(service, node);
851-
});
880+
auto js_obj =
881+
RclHandle::NewInstance(service, node_handle, [node](void* ptr) {
882+
rcl_service_t* service = reinterpret_cast<rcl_service_t*>(ptr);
883+
rcl_ret_t ret = rcl_service_fini(service, node);
884+
free(ptr);
885+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
886+
});
852887

853888
info.GetReturnValue().Set(js_obj);
854889
} else {
@@ -868,7 +903,8 @@ NAN_METHOD(RclTakeRequest) {
868903
node::Buffer::Data(Nan::To<v8::Object>(info[2]).ToLocalChecked());
869904
rcl_ret_t ret = rcl_take_request(service, header, taken_request);
870905
if (ret != RCL_RET_SERVICE_TAKE_FAILED) {
871-
auto js_obj = RclHandle::NewInstance(header);
906+
auto js_obj =
907+
RclHandle::NewInstance(header, nullptr, [](void* ptr) { free(ptr); });
872908
info.GetReturnValue().Set(js_obj);
873909
return;
874910
}
@@ -1294,7 +1330,7 @@ NAN_METHOD(CreateArrayBufferCleaner) {
12941330

12951331
char* target = *reinterpret_cast<char**>(address + offset);
12961332
info.GetReturnValue().Set(
1297-
RclHandle::NewInstance(target, nullptr, [] { return RCL_RET_OK; }));
1333+
RclHandle::NewInstance(target, nullptr, [](void* ptr) { free(ptr); }));
12981334
}
12991335

13001336
NAN_METHOD(setLoggerLevel) {
@@ -1378,8 +1414,12 @@ NAN_METHOD(CreateContext) {
13781414
rcl_context_t* context =
13791415
reinterpret_cast<rcl_context_t*>(malloc(sizeof(rcl_context_t)));
13801416
*context = rcl_get_zero_initialized_context();
1381-
auto js_obj = RclHandle::NewInstance(context, nullptr,
1382-
std::bind(DestroyContext, context));
1417+
auto js_obj = RclHandle::NewInstance(context, nullptr, [](void* ptr) {
1418+
rcl_context_t* context = reinterpret_cast<rcl_context_t*>(ptr);
1419+
rcl_ret_t ret = DestroyContext(context);
1420+
free(ptr);
1421+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK, ret, rcl_get_error_string().str);
1422+
});
13831423

13841424
info.GetReturnValue().Set(js_obj);
13851425
}

‎src/rcl_handle.cpp‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ NAN_METHOD(RclHandle::Dismiss) {
8585
info.GetReturnValue().Set(Nan::Undefined());
8686
}
8787

88-
v8::Local<v8::Object> RclHandle::NewInstance(void* handle, RclHandle* parent,
89-
std::function<int()> deleter) {
88+
v8::Local<v8::Object> RclHandle::NewInstance(
89+
void* handle, RclHandle* parent, std::function<void(void*)> deleter) {
9090
Nan::EscapableHandleScope scope;
9191

9292
v8::Local<v8::Function> cons = Nan::New<v8::Function>(constructor);
@@ -123,12 +123,7 @@ void RclHandle::Reset() {
123123
child->Reset();
124124
}
125125

126-
if (deleter_ && deleter_() != RCL_RET_OK) {
127-
Nan::ThrowError(rcl_get_error_string().str);
128-
rcl_reset_error();
129-
}
130-
131-
free(pointer_);
126+
if (deleter_) deleter_(pointer_);
132127

133128
pointer_ = nullptr;
134129
children_.clear();

‎src/rcl_handle.hpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,10 @@ namespace rclnodejs {
2727
class RclHandle : public Nan::ObjectWrap {
2828
public:
2929
static void Init(v8::Local<v8::Object> exports);
30-
static v8::Local<v8::Object> NewInstance(
31-
void* handle, RclHandle* parent = nullptr,
32-
std::function<int()> deleter = [] { return 0; } );
30+
static v8::Local<v8::Object> NewInstance(void* handle, RclHandle* parent,
31+
std::function<void(void*)> deleter);
3332

34-
void set_deleter(std::function<int()> deleter) { deleter_ = deleter; }
33+
void set_deleter(std::function<void(void*)> deleter) { deleter_ = deleter; }
3534

3635
RclHandle* parent() { return parent_; }
3736
void set_parent(RclHandle* parent) { parent_ = parent; }
@@ -63,7 +62,7 @@ class RclHandle : public Nan::ObjectWrap {
6362
std::map<std::string, bool> properties_;
6463
v8::Local<v8::Object> properties_obj_;
6564

66-
std::function<int()> deleter_;
65+
std::function<void(void*)> deleter_;
6766
std::set<RclHandle*> children_;
6867
};
6968

0 commit comments

Comments
 (0)