Fix: resolve memory leaks, use-after-free, and correctness issues in … (#1408)

Comprehensive audit and fix of all C++ source files under `src/` that implement
N-API bindings for the ROS2 rcl library. Five categories of issues were
identified and resolved.

### 1. Use-after-free in context deleter lambda

**File:** `src/rcl_context_bindings.cpp`

Changed `[&env]` capture to `[env]` in the lambda deleter passed to
`RclHandle`. Since `Napi::Env` is a lightweight pointer wrapper, capturing by
value copies the `napi_env` pointer safely. Capturing by reference created a
dangling reference because the lambda outlives the stack frame where `env` was
declared — the deleter runs during garbage collection, long after
`CreateContext` returns.

### 2. Memory leaks on early-return error paths

**Files:** 9 binding files

When `rcl_*_init()` fails, the previously `malloc`'d handle was not freed
before throwing and returning. Added `free()` calls before the error throw in:

- `src/rcl_node_bindings.cpp` — `CreateNode`
- `src/rcl_publisher_bindings.cpp` — `CreatePublisher`
- `src/rcl_timer_bindings.cpp` — `CreateTimer` (both `ROS_VERSION > 2305` branches)
- `src/rcl_guard_condition_bindings.cpp` — `CreateGuardCondition`
- `src/rcl_client_bindings.cpp` — `CreateClient`
- `src/rcl_service_bindings.cpp` — `CreateService`
- `src/rcl_action_client_bindings.cpp` — `ActionCreateClient`
- `src/rcl_action_server_bindings.cpp` — `ActionCreateServer`
- `src/rcl_lifecycle_bindings.cpp` — `CreateLifecycleStateMachine` (all 3 version branches)

### 3. Missing return after throw

**File:** `src/rcl_type_description_service_bindings.cpp`

Added `free(service)` and `return env.Undefined()` after
`ThrowAsJavaScriptException()` on init failure. Without the return, execution
continued and wrapped the uninitialized handle into an `RclHandle`, which would
later attempt to finalize invalid memory.

### 4. Static variable not reset between calls

**File:** `src/executor.cpp`

Added `handle_closed = false;` immediately after the `static bool handle_closed`
declaration in `Executor::Stop()`. Static local variables are initialized only
once; on a second call to `Stop()`, `handle_closed` would still be `true` from
the previous invocation, causing the `while (!handle_closed)` loop to be
skipped entirely — the `uv_close` callback would never run and the async handle
would leak.

Fix: #1407

Author: minggangw

src/executor.cpp

@@ -110,6 +110,7 @@ void Executor::Stop() {
 
     if (uv_is_active(reinterpret_cast<uv_handle_t*>(async_))) {
       static bool handle_closed = false;
+      handle_closed = false;
       uv_close(reinterpret_cast<uv_handle_t*>(async_),
                [](uv_handle_t* async) -> void {
                  // Important Notice:

src/rcl_action_client_bindings.cpp

@@ -70,10 +70,17 @@ Napi::Value ActionCreateClient(const Napi::CallbackInfo& info) {
         malloc(sizeof(rcl_action_client_t)));
     *action_client = rcl_action_get_zero_initialized_client();
 
-    THROW_ERROR_IF_NOT_EQUAL(
-        rcl_action_client_init(action_client, node, ts, action_name.c_str(),
-                               &action_client_ops),
-        RCL_RET_OK, rcl_get_error_string().str);
+    {
+      rcl_ret_t ret = rcl_action_client_init(
+          action_client, node, ts, action_name.c_str(), &action_client_ops);
+      if (RCL_RET_OK != ret) {
+        std::string error_msg = rcl_get_error_string().str;
+        rcl_reset_error();
+        free(action_client);
+        Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
+        return env.Undefined();
+      }
+    }
     auto js_obj = RclHandle::NewInstance(
         env, action_client, node_handle, [node, env](void* ptr) {
           rcl_action_client_t* action_client =

src/rcl_action_server_bindings.cpp

@@ -76,10 +76,18 @@ Napi::Value ActionCreateServer(const Napi::CallbackInfo& info) {
         malloc(sizeof(rcl_action_server_t)));
     *action_server = rcl_action_get_zero_initialized_server();
 
-    THROW_ERROR_IF_NOT_EQUAL(
-        rcl_action_server_init(action_server, node, clock, ts,
-                               action_name.c_str(), &action_server_ops),
-        RCL_RET_OK, rcl_get_error_string().str);
+    {
+      rcl_ret_t ret =
+          rcl_action_server_init(action_server, node, clock, ts,
+                                 action_name.c_str(), &action_server_ops);
+      if (RCL_RET_OK != ret) {
+        std::string error_msg = rcl_get_error_string().str;
+        rcl_reset_error();
+        free(action_server);
+        Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
+        return env.Undefined();
+      }
+    }
     auto js_obj = RclHandle::NewInstance(
         env, action_server, node_handle, [node, env](void* ptr) {
           rcl_action_server_t* action_server =

src/rcl_client_bindings.cpp

@@ -49,9 +49,17 @@ Napi::Value CreateClient(const Napi::CallbackInfo& info) {
       client_ops.qos = *qos_profile;
     }
 
-    THROW_ERROR_IF_NOT_EQUAL(
-        rcl_client_init(client, node, ts, service_name.c_str(), &client_ops),
-        RCL_RET_OK, rcl_get_error_string().str);
+    {
+      rcl_ret_t ret =
+          rcl_client_init(client, node, ts, service_name.c_str(), &client_ops);
+      if (RCL_RET_OK != ret) {
+        std::string error_msg = rcl_get_error_string().str;
+        rcl_reset_error();
+        free(client);
+        Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
+        return env.Undefined();
+      }
+    }
 
     auto js_obj = RclHandle::NewInstance(
         env, client, node_handle, [node, env](void* ptr) {

src/rcl_context_bindings.cpp

@@ -120,14 +120,13 @@ Napi::Value CreateContext(const Napi::CallbackInfo& info) {
   rcl_context_t* context =
       reinterpret_cast<rcl_context_t*>(malloc(sizeof(rcl_context_t)));
   *context = rcl_get_zero_initialized_context();
-  auto js_obj =
-      RclHandle::NewInstance(env, context, nullptr, [&env](void* ptr) {
-        rcl_context_t* context = reinterpret_cast<rcl_context_t*>(ptr);
-        rcl_ret_t ret = DestroyContext(env, context);
-        free(ptr);
-        THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
-                                           rcl_get_error_string().str);
-      });
+  auto js_obj = RclHandle::NewInstance(env, context, nullptr, [env](void* ptr) {
+    rcl_context_t* context = reinterpret_cast<rcl_context_t*>(ptr);
+    rcl_ret_t ret = DestroyContext(env, context);
+    free(ptr);
+    THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
+                                       rcl_get_error_string().str);
+  });
 
   return js_obj;
 }

src/rcl_guard_condition_bindings.cpp

@@ -17,6 +17,8 @@
 #include <rcl/error_handling.h>
 #include <rcl/rcl.h>
 
+#include <string>
+
 #include "macros.h"
 #include "rcl_handle.h"
 #include "rcl_utilities.h"
@@ -37,9 +39,16 @@ Napi::Value CreateGuardCondition(const Napi::CallbackInfo& info) {
   rcl_guard_condition_options_t gc_options =
       rcl_guard_condition_get_default_options();
 
-  THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK,
-                           rcl_guard_condition_init(gc, context, gc_options),
-                           rcl_get_error_string().str);
+  {
+    rcl_ret_t ret = rcl_guard_condition_init(gc, context, gc_options);
+    if (RCL_RET_OK != ret) {
+      std::string error_msg = rcl_get_error_string().str;
+      rcl_reset_error();
+      free(gc);
+      Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
+      return env.Undefined();
+    }
+  }
 
   auto handle = RclHandle::NewInstance(env, gc, nullptr, [env](void* ptr) {
     rcl_guard_condition_t* gc = reinterpret_cast<rcl_guard_condition_t*>(ptr);

src/rcl_lifecycle_bindings.cpp

@@ -79,11 +79,17 @@ Napi::Value CreateLifecycleStateMachine(const Napi::CallbackInfo& info) {
   RclHandle* clock_handle = RclHandle::Unwrap(info[2].As<Napi::Object>());
   rcl_clock_t* clock = reinterpret_cast<rcl_clock_t*>(clock_handle->ptr());
 
-  THROW_ERROR_IF_NOT_EQUAL(
-      RCL_RET_OK,
-      rcl_lifecycle_state_machine_init(state_machine, node, clock, pn, cs, gs,
-                                       gas, gat, gtg, &options),
-      rcl_get_error_string().str);
+  {
+    rcl_ret_t ret = rcl_lifecycle_state_machine_init(
+        state_machine, node, clock, pn, cs, gs, gas, gat, gtg, &options);
+    if (RCL_RET_OK != ret) {
+      std::string error_msg = rcl_get_error_string().str;
+      rcl_reset_error();
+      free(state_machine);
+      Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
+      return env.Undefined();
+    }
+  }
 
   auto js_obj = RclHandle::NewInstance(
       env, state_machine, node_handle, [node, env](void* ptr) {
@@ -99,11 +105,17 @@ Napi::Value CreateLifecycleStateMachine(const Napi::CallbackInfo& info) {
       rcl_lifecycle_get_default_state_machine_options();
   options.enable_com_interface = info[1].As<Napi::Boolean>().Value();
 
-  THROW_ERROR_IF_NOT_EQUAL(
-      RCL_RET_OK,
-      rcl_lifecycle_state_machine_init(state_machine, node, pn, cs, gs, gas,
-                                       gat, gtg, &options),
-      rcl_get_error_string().str);
+  {
+    rcl_ret_t ret = rcl_lifecycle_state_machine_init(
+        state_machine, node, pn, cs, gs, gas, gat, gtg, &options);
+    if (RCL_RET_OK != ret) {
+      std::string error_msg = rcl_get_error_string().str;
+      rcl_reset_error();
+      free(state_machine);
+      Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
+      return env.Undefined();
+    }
+  }
 
   auto js_obj = RclHandle::NewInstance(
       env, state_machine, node_handle, [node, env](void* ptr) {
@@ -118,11 +130,18 @@ Napi::Value CreateLifecycleStateMachine(const Napi::CallbackInfo& info) {
   const rcl_node_options_t* node_options =
       reinterpret_cast<const rcl_node_options_t*>(rcl_node_get_options(node));
 
-  THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK,
-                           rcl_lifecycle_state_machine_init(
-                               state_machine, node, pn, cs, gs, gas, gat, gtg,
-                               true, &node_options->allocator),
-                           rcl_get_error_string().str);
+  {
+    rcl_ret_t ret = rcl_lifecycle_state_machine_init(
+        state_machine, node, pn, cs, gs, gas, gat, gtg, true,
+        &node_options->allocator);
+    if (RCL_RET_OK != ret) {
+      std::string error_msg = rcl_get_error_string().str;
+      rcl_reset_error();
+      free(state_machine);
+      Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
+      return env.Undefined();
+    }
+  }
 
   auto js_obj = RclHandle::NewInstance(
       env, state_machine, node_handle, [node, node_options, env](void* ptr) {

src/rcl_node_bindings.cpp

@@ -224,10 +224,17 @@ Napi::Value CreateNode(const Napi::CallbackInfo& info) {
     options.rosout_qos = *qos_profile;
   }
 
-  THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK,
-                           rcl_node_init(node, node_name.c_str(),
-                                         name_space.c_str(), context, &options),
-                           rcl_get_error_string().str);
+  {
+    rcl_ret_t ret = rcl_node_init(node, node_name.c_str(), name_space.c_str(),
+                                  context, &options);
+    if (RCL_RET_OK != ret) {
+      std::string error_msg = rcl_get_error_string().str;
+      rcl_reset_error();
+      free(node);
+      Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
+      return env.Undefined();
+    }
+  }
 
   auto handle = RclHandle::NewInstance(env, node, nullptr, [env](void* ptr) {
     rcl_node_t* node = reinterpret_cast<rcl_node_t*>(ptr);

src/rcl_publisher_bindings.cpp

@@ -51,9 +51,17 @@ Napi::Value CreatePublisher(const Napi::CallbackInfo& info) {
       publisher_ops.qos = *qos_profile;
     }
 
-    THROW_ERROR_IF_NOT_EQUAL(
-        rcl_publisher_init(publisher, node, ts, topic.c_str(), &publisher_ops),
-        RCL_RET_OK, rcl_get_error_string().str);
+    {
+      rcl_ret_t ret = rcl_publisher_init(publisher, node, ts, topic.c_str(),
+                                         &publisher_ops);
+      if (RCL_RET_OK != ret) {
+        std::string error_msg = rcl_get_error_string().str;
+        rcl_reset_error();
+        free(publisher);
+        Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
+        return env.Undefined();
+      }
+    }
 
     auto js_obj = RclHandle::NewInstance(
         env, publisher, node_handle, [node, env](void* ptr) {
@@ -66,6 +74,7 @@ Napi::Value CreatePublisher(const Napi::CallbackInfo& info) {
 
     return js_obj;
   } else {
+    free(publisher);
     Napi::Error::New(env, GetErrorMessageAndClear())
         .ThrowAsJavaScriptException();
     return env.Undefined();

src/rcl_service_bindings.cpp

@@ -52,9 +52,17 @@ Napi::Value CreateService(const Napi::CallbackInfo& info) {
       service_ops.qos = *qos_profile;
     }
 
-    THROW_ERROR_IF_NOT_EQUAL(
-        rcl_service_init(service, node, ts, service_name.c_str(), &service_ops),
-        RCL_RET_OK, rcl_get_error_string().str);
+    {
+      rcl_ret_t ret = rcl_service_init(service, node, ts, service_name.c_str(),
+                                       &service_ops);
+      if (RCL_RET_OK != ret) {
+        std::string error_msg = rcl_get_error_string().str;
+        rcl_reset_error();
+        free(service);
+        Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
+        return env.Undefined();
+      }
+    }
     auto js_obj = RclHandle::NewInstance(
         env, service, node_handle, [node, env](void* ptr) {
           rcl_service_t* service = reinterpret_cast<rcl_service_t*>(ptr);