RobotWebTools / rclnodejs / 22838687130

86%

push

github

Fix memory leaks, error handling, and type-safety issues in N-API bindings (#1422)

### Memory leaks (Critical)

Added missing `free(header)` on `TAKE_FAILED` early-return paths where `malloc`'d `rmw_request_id_t*` was never freed.
- `src/rcl_action_server_bindings.cpp` — `ActionTakeResultRequest`, `ActionTakeGoalRequest`, `ActionTakeCancelRequest`
- `src/rcl_service_bindings.cpp` — `RclTakeRequest`

### Error handling misordering (High)

Captured `rcl_get_error_string()` before calling `rcl_reset_error()` to avoid passing empty error messages to JavaScript.
- `src/rcl_action_client_bindings.cpp` — `ActionTakeFeedback`, `ActionTakeStatus`
- `src/rcl_action_server_bindings.cpp` — `ActionTakeGoalResponse`, `ActionTakeCancelResponse`, `ActionTakeResultResponse`

### Sequence number truncation (High)

Changed `int32_t`/`uint32_t` → `double` casts for 64-bit sequence numbers to avoid silent truncation above 2^31.
- `src/rcl_action_client_bindings.cpp`, `src/rcl_action_server_bindings.cpp`, `src/rcl_client_bindings.cpp`

### Exception-safe cleanup (Medium)

Used `RCPPUTILS_SCOPE_EXIT` in `src/rcl_subscription_bindings.cpp` for `RclTakeRaw` and `GetContentFilter` to ensure resource finalization even if N-API calls throw. Both scope-exit blocks check the fini return value and call `rcl_reset_error()` on failure to prevent stale error state from leaking into subsequent calls. Reordered `free(subscription)` before `ThrowAsJavaScriptException()` in `CreateSubscription`.

### Context rollback on init failure (Medium)

Wrapped post-init steps in `index.js` with `try/catch` that calls `context.tryShutdown()` if `generator.generateAll()` fails, preventing a half-initialized context from blocking re-initialization. The catch block normalizes non-Error exceptions before appending rollback failure details. Added corresponding test in `test/test-init-shutdown.js` with `try/finally` to guarantee sinon stub restoration even on assertion failure.

Fix: #1420

1390 of 1756 branches covered (79.16%)

Branch coverage included in aggregate %.

2871 of 3204 relevant lines covered (89.61%)

449.84 hits per line