From 75495f26e7718da7d6f9e0ad06af1caa2e3ebf08 Mon Sep 17 00:00:00 2001 From: tpantelis Date: Mon, 8 Dec 2014 09:25:16 -0500 Subject: [PATCH 1/1] Bug 2517: Catch RuntimeExceptions thrown from the DCL in DataChangeListener This prevents akka from suppressing/dropping subsequent data change events if a RuntimeException is propagated to akka. Change-Id: Ibc90e00c56a3e3b6f0039ff609d639fe8201adf0 Signed-off-by: tpantelis --- .../cluster/datastore/DataChangeListener.java | 6 +++- .../datastore/DataChangeListenerTest.java | 35 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DataChangeListener.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DataChangeListener.java index 9a77e4d568..6f14af304f 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DataChangeListener.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DataChangeListener.java @@ -62,7 +62,11 @@ public class DataChangeListener extends AbstractUntypedActor { LOG.debug("Sending change notification {} to listener {}", change, listener); - this.listener.onDataChanged(change); + try { + this.listener.onDataChanged(change); + } catch (RuntimeException e) { + LOG.error( String.format( "Error notifying listener %s", this.listener ), e ); + } // It seems the sender is never null but it doesn't hurt to check. If the caller passes in // a null sender (ActorRef.noSender()), akka translates that to the deadLetters actor. diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DataChangeListenerTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DataChangeListenerTest.java index d5a12c73c5..25d47388fe 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DataChangeListenerTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DataChangeListenerTest.java @@ -13,6 +13,7 @@ import org.opendaylight.controller.cluster.datastore.messages.EnableNotification import org.opendaylight.controller.md.cluster.datastore.model.CompositeModel; import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeEvent; import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeListener; +import org.opendaylight.yangtools.yang.model.api.SchemaContext; public class DataChangeListenerTest extends AbstractActorTest { @@ -92,4 +93,38 @@ public class DataChangeListenerTest extends AbstractActorTest { } }}; } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + @Test + public void testDataChangedWithListenerRuntimeEx(){ + new JavaTestKit(getSystem()) {{ + AsyncDataChangeEvent mockChangeEvent1 = Mockito.mock(AsyncDataChangeEvent.class); + AsyncDataChangeEvent mockChangeEvent2 = Mockito.mock(AsyncDataChangeEvent.class); + AsyncDataChangeEvent mockChangeEvent3 = Mockito.mock(AsyncDataChangeEvent.class); + + AsyncDataChangeListener mockListener = Mockito.mock(AsyncDataChangeListener.class); + Mockito.doThrow(new RuntimeException("mock")).when(mockListener).onDataChanged(mockChangeEvent2); + + Props props = DataChangeListener.props(mockListener); + ActorRef subject = getSystem().actorOf(props, "testDataChangedWithListenerRuntimeEx"); + + // Let the DataChangeListener know that notifications should be enabled + subject.tell(new EnableNotification(true), getRef()); + + SchemaContext schemaContext = CompositeModel.createTestContext(); + + subject.tell(new DataChanged(schemaContext, mockChangeEvent1),getRef()); + expectMsgClass(DataChangedReply.class); + + subject.tell(new DataChanged(schemaContext, mockChangeEvent2),getRef()); + expectMsgClass(DataChangedReply.class); + + subject.tell(new DataChanged(schemaContext, mockChangeEvent3),getRef()); + expectMsgClass(DataChangedReply.class); + + Mockito.verify(mockListener).onDataChanged(mockChangeEvent1); + Mockito.verify(mockListener).onDataChanged(mockChangeEvent2); + Mockito.verify(mockListener).onDataChanged(mockChangeEvent3); + }}; + } } -- 2.36.6