From bb403a3f97313462cd694431a641e7ba283c5d11 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Thu, 20 Aug 2020 18:18:43 -0300 Subject: [PATCH 1/3] Autoremove EventHandler from parent Signed-off-by: Ivan Santiago Paunovic --- .../org/ros2/rcljava/events/EventHandlerImpl.java | 15 +++++++++++---- .../org/ros2/rcljava/publisher/PublisherImpl.java | 12 +++++++++++- .../rcljava/subscription/SubscriptionImpl.java | 13 ++++++++++++- .../org/ros2/rcljava/publisher/PublisherTest.java | 5 ++++- 4 files changed, 38 insertions(+), 7 deletions(-) diff --git a/rcljava/src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java b/rcljava/src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java index 25545c7f..ffa8cb7b 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java @@ -20,6 +20,7 @@ import org.ros2.rcljava.common.JNIUtils; import org.ros2.rcljava.consumers.Consumer; +import org.ros2.rcljava.consumers.Consumer; import org.ros2.rcljava.events.EventHandler; import org.ros2.rcljava.events.EventStatus; import org.ros2.rcljava.interfaces.Disposable; @@ -64,16 +65,19 @@ public class EventHandlerImpl< * @param eventStatusFactory Factory of an event status. * @param callback The callback function that will be called when the event * is triggered. + * @param removeCallback Callback that will be called when this object is being disposed. */ public EventHandlerImpl( final WeakReference parentReference, final long handle, final Supplier eventStatusFactory, - final Consumer callback) { + final Consumer callback, + final Consumer removeCallback) { this.parentReference = parentReference; this.handle = handle; this.eventStatusFactory = eventStatusFactory; this.callback = callback; + this.removeCallback = removeCallback; } /** @@ -102,9 +106,11 @@ public final long getHandle() { * {@inheritDoc} */ public synchronized final void dispose() { - if (this.handle != 0) { - nativeDispose(this.handle); + if (this.handle == 0) { + return; } + this.removeCallback.accept(this); + nativeDispose(this.handle); this.handle = 0; } @@ -126,11 +132,12 @@ public synchronized final void executeCallback() { nativeTake(this.handle, nativeEventStatusHandle); eventStatus.fromRCLEvent(nativeEventStatusHandle); eventStatus.deallocateRCLStatusEvent(nativeEventStatusHandle); - callback.accept(eventStatus); + this.callback.accept(eventStatus); } private final Supplier eventStatusFactory; private final WeakReference parentReference; private long handle; private final Consumer callback; + private final Consumer removeCallback; } 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 17018f87..ed347188 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/publisher/PublisherImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/publisher/PublisherImpl.java @@ -117,10 +117,20 @@ public final WeakReference getNodeReference() { public final EventHandler createEventHandler(Supplier factory, Consumer callback) { + final WeakReference> weakEventHandlers = new WeakReference( + this.eventHandlers); + Consumer removeCallback = new Consumer() { + public void accept(EventHandler eventHandler) { + Collection eventHandlers = weakEventHandlers.get(); + if (eventHandlers != null) { + eventHandlers.remove(eventHandler); + } + } + }; T status = factory.get(); long eventHandle = nativeCreateEvent(this.handle, status.getPublisherEventType()); EventHandler eventHandler = new EventHandlerImpl( - new WeakReference(this), eventHandle, factory, callback); + new WeakReference(this), eventHandle, factory, callback, removeCallback); this.eventHandlers.add(eventHandler); return eventHandler; } 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 34b8b79c..5681ca8f 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/subscription/SubscriptionImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/subscription/SubscriptionImpl.java @@ -22,6 +22,7 @@ import org.ros2.rcljava.RCLJava; import org.ros2.rcljava.common.JNIUtils; +import org.ros2.rcljava.consumers.BiConsumer; import org.ros2.rcljava.consumers.Consumer; import org.ros2.rcljava.events.EventHandler; import org.ros2.rcljava.events.EventHandlerImpl; @@ -128,10 +129,20 @@ public final WeakReference getNodeReference() { public final EventHandler createEventHandler(Supplier factory, Consumer callback) { + final WeakReference> weakEventHandlers = new WeakReference( + this.eventHandlers); + Consumer removeCallback = new Consumer() { + public void accept(EventHandler eventHandler) { + Collection eventHandlers = weakEventHandlers.get(); + if (eventHandlers != null) { + eventHandlers.remove(eventHandler); + } + } + }; T status = factory.get(); long eventHandle = nativeCreateEvent(this.handle, status.getSubscriptionEventType()); EventHandler eventHandler = new EventHandlerImpl( - new WeakReference(this), eventHandle, factory, callback); + new WeakReference(this), eventHandle, factory, callback, removeCallback); this.eventHandlers.add(eventHandler); return eventHandler; } diff --git a/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java b/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java index 7ee84de8..3890774a 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java @@ -90,9 +90,12 @@ public void accept(final OfferedQosIncompatible status) { } ); assertNotEquals(0, eventHandler.getHandle()); + assertEquals(1, publisher.getEventHandlers().size()); // force executing the callback, so we check that taking an event works eventHandler.executeCallback(); - RCLJava.shutdown(); + eventHandler.dispose(); assertEquals(0, eventHandler.getHandle()); + assertEquals(0, publisher.getEventHandlers().size()); + RCLJava.shutdown(); } } From a0335cc19976fdd95b2d294eab2d65a45a5ddb50 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Wed, 26 Aug 2020 11:35:40 -0300 Subject: [PATCH 2/3] Remove duplicated import Signed-off-by: Ivan Santiago Paunovic --- .../src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/rcljava/src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java b/rcljava/src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java index ffa8cb7b..2b1a1efa 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java @@ -20,7 +20,6 @@ import org.ros2.rcljava.common.JNIUtils; import org.ros2.rcljava.consumers.Consumer; -import org.ros2.rcljava.consumers.Consumer; import org.ros2.rcljava.events.EventHandler; import org.ros2.rcljava.events.EventStatus; import org.ros2.rcljava.interfaces.Disposable; From f1c70c3767399a7b7db7f80b28b303a0b60aef81 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Wed, 26 Aug 2020 17:34:57 -0300 Subject: [PATCH 3/3] Rename removeCallback as disposeCallback Signed-off-by: Ivan Santiago Paunovic --- .../java/org/ros2/rcljava/events/EventHandlerImpl.java | 10 +++++----- .../java/org/ros2/rcljava/publisher/PublisherImpl.java | 4 ++-- .../ros2/rcljava/subscription/SubscriptionImpl.java | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rcljava/src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java b/rcljava/src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java index 2b1a1efa..74e4e6dd 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/events/EventHandlerImpl.java @@ -64,19 +64,19 @@ public class EventHandlerImpl< * @param eventStatusFactory Factory of an event status. * @param callback The callback function that will be called when the event * is triggered. - * @param removeCallback Callback that will be called when this object is being disposed. + * @param disposeCallback Callback that will be called when this object is disposed. */ public EventHandlerImpl( final WeakReference parentReference, final long handle, final Supplier eventStatusFactory, final Consumer callback, - final Consumer removeCallback) { + final Consumer disposeCallback) { this.parentReference = parentReference; this.handle = handle; this.eventStatusFactory = eventStatusFactory; this.callback = callback; - this.removeCallback = removeCallback; + this.disposeCallback = disposeCallback; } /** @@ -108,7 +108,7 @@ public synchronized final void dispose() { if (this.handle == 0) { return; } - this.removeCallback.accept(this); + this.disposeCallback.accept(this); nativeDispose(this.handle); this.handle = 0; } @@ -138,5 +138,5 @@ public synchronized final void executeCallback() { private final WeakReference parentReference; private long handle; private final Consumer callback; - private final Consumer removeCallback; + private final Consumer disposeCallback; } 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 ed347188..0e93531e 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/publisher/PublisherImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/publisher/PublisherImpl.java @@ -119,7 +119,7 @@ public final WeakReference getNodeReference() { createEventHandler(Supplier factory, Consumer callback) { final WeakReference> weakEventHandlers = new WeakReference( this.eventHandlers); - Consumer removeCallback = new Consumer() { + Consumer disposeCallback = new Consumer() { public void accept(EventHandler eventHandler) { Collection eventHandlers = weakEventHandlers.get(); if (eventHandlers != null) { @@ -130,7 +130,7 @@ public void accept(EventHandler eventHandler) { T status = factory.get(); long eventHandle = nativeCreateEvent(this.handle, status.getPublisherEventType()); EventHandler eventHandler = new EventHandlerImpl( - new WeakReference(this), eventHandle, factory, callback, removeCallback); + new WeakReference(this), eventHandle, factory, callback, disposeCallback); this.eventHandlers.add(eventHandler); return eventHandler; } 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 5681ca8f..8e20d81d 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/subscription/SubscriptionImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/subscription/SubscriptionImpl.java @@ -131,7 +131,7 @@ public final WeakReference getNodeReference() { createEventHandler(Supplier factory, Consumer callback) { final WeakReference> weakEventHandlers = new WeakReference( this.eventHandlers); - Consumer removeCallback = new Consumer() { + Consumer disposeCallback = new Consumer() { public void accept(EventHandler eventHandler) { Collection eventHandlers = weakEventHandlers.get(); if (eventHandlers != null) { @@ -142,7 +142,7 @@ public void accept(EventHandler eventHandler) { T status = factory.get(); long eventHandle = nativeCreateEvent(this.handle, status.getSubscriptionEventType()); EventHandler eventHandler = new EventHandlerImpl( - new WeakReference(this), eventHandle, factory, callback, removeCallback); + new WeakReference(this), eventHandle, factory, callback, disposeCallback); this.eventHandlers.add(eventHandler); return eventHandler; }