From 744498322358b584d2f6c49cb9d05d629a0b769b Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Fri, 18 Sep 2020 11:24:15 -0700 Subject: [PATCH] Log error if calls to dispose fail In these cases, we are likely leaking memory. Signed-off-by: Jacob Perron --- .../main/java/org/ros2/rcljava/client/ClientImpl.java | 10 ++++++---- .../java/org/ros2/rcljava/publisher/PublisherImpl.java | 10 ++++++---- .../java/org/ros2/rcljava/service/ServiceImpl.java | 10 ++++++---- .../ros2/rcljava/subscription/SubscriptionImpl.java | 10 ++++++---- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/rcljava/src/main/java/org/ros2/rcljava/client/ClientImpl.java b/rcljava/src/main/java/org/ros2/rcljava/client/ClientImpl.java index 5597f005..1269de83 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/client/ClientImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/client/ClientImpl.java @@ -126,11 +126,13 @@ public final Class getResponseType() { */ public final void dispose() { Node node = this.nodeReference.get(); - if (node != null) { - node.removeClient(this); - nativeDispose(node.getHandle(), this.handle); - this.handle = 0; + if (node == null) { + logger.error("Node reference is null. Failed to dispose of Client."); + return; } + node.removeClient(this); + nativeDispose(node.getHandle(), this.handle); + this.handle = 0; } /** diff --git a/rcljava/src/main/java/org/ros2/rcljava/publisher/PublisherImpl.java b/rcljava/src/main/java/org/ros2/rcljava/publisher/PublisherImpl.java index 0e93531e..7911916b 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/publisher/PublisherImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/publisher/PublisherImpl.java @@ -187,10 +187,12 @@ public final void dispose() { } this.eventHandlers.clear(); Node node = this.nodeReference.get(); - if (node != null) { - node.removePublisher(this); - nativeDispose(node.getHandle(), this.handle); - this.handle = 0; + if (node == null) { + logger.error("Node reference is null. Failed to dispose of Publisher."); + return; } + node.removePublisher(this); + nativeDispose(node.getHandle(), this.handle); + this.handle = 0; } } diff --git a/rcljava/src/main/java/org/ros2/rcljava/service/ServiceImpl.java b/rcljava/src/main/java/org/ros2/rcljava/service/ServiceImpl.java index e45d06ec..0257813a 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/service/ServiceImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/service/ServiceImpl.java @@ -84,11 +84,13 @@ public final Class getResponseType() { */ public final void dispose() { Node node = this.nodeReference.get(); - if (node != null) { - node.removeService(this); - nativeDispose(node.getHandle(), this.handle); - this.handle = 0; + if (node == null) { + logger.error("Node reference is null. Failed to dispose of Service."); + return; } + node.removeService(this); + nativeDispose(node.getHandle(), this.handle); + this.handle = 0; } /** diff --git a/rcljava/src/main/java/org/ros2/rcljava/subscription/SubscriptionImpl.java b/rcljava/src/main/java/org/ros2/rcljava/subscription/SubscriptionImpl.java index 8e20d81d..7a969af0 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/subscription/SubscriptionImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/subscription/SubscriptionImpl.java @@ -200,11 +200,13 @@ public final void dispose() { } this.eventHandlers.clear(); Node node = this.nodeReference.get(); - if (node != null) { - node.removeSubscription(this); - nativeDispose(node.getHandle(), this.handle); - this.handle = 0; + if (node == null) { + logger.error("Node reference is null. Failed to dispose of Subscription."); + return; } + node.removeSubscription(this); + nativeDispose(node.getHandle(), this.handle); + this.handle = 0; } public void executeCallback(T message) {