Fix findbugs violations in md-sal - part 2 27/69327/3
authorTom Pantelis <tompantelis@gmail.com>
Thu, 8 Mar 2018 22:41:36 +0000 (17:41 -0500)
committerMichael Vorburger <vorburger@redhat.com>
Thu, 15 Mar 2018 21:45:26 +0000 (21:45 +0000)
- sal-broker-impl
- sal-dom-broker-config
- sal-binding-config
- sal-akka-raft-example
- clustering-it-provider

Violations:

- Method may return null, but is declared @Nonnull
- Method with Optional return type returns explicit null
- Method ignores exceptional return value
- Field not guarded against concurrent access
- Unchecked/unconfirmed cast of return value from method
- Load of known null value
- Parameter must be non-null but is marked as nullable
- Class implements same interface as superclass
- Redundant nullcheck of value known to be non-null
- Unread field
- Reliance on default encoding
- Should be a static inner class
- Questionable cast to concrete collection
- Dead store to local variable
- Dereference of the result of readLine() without nullcheck
- Method ignores return value
- Finalizer does not call superclass finalizer
- An increment to a volatile field isn't atomic
- Dead store to local variable
- Redundant nullcheck of value known to be non-null

Change-Id: Iec7205b49a0cbafff33db97c9c753a5425f929a6
Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
21 files changed:
opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/ExampleActor.java
opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/ExampleRoleChangeListener.java
opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/LogGenerator.java
opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/Main.java
opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/TestDriver.java
opendaylight/md-sal/sal-binding-config/pom.xml
opendaylight/md-sal/sal-dom-broker-config/pom.xml
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/AbstractDOMDataBroker.java
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMNotificationRouter.java
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMNotificationRouterEvent.java
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/PingPongFuture.java
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/TransactionCommitFailedExceptionMapper.java
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/legacy/sharded/adapter/ShardedDOMDataBrokerDelegatingReadWriteTransaction.java
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/clustering/impl/LegacyEntityOwnershipServiceAdapter.java
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/ConsumerContextImpl.java
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/osgi/ProxyFactory.java
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/util/YangSchemaUtils.java [deleted file]
opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/CarProvider.java
opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/MdsalLowLevelTestProvider.java
opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/FinalizableScheduledExecutorService.java
opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/FlappingSingletonService.java

index 11278ede10012698b4bc1950be1bd0d76f61291a..a1916f15002113c2e3a900b7b7be6cfa22c8b35c 100644 (file)
@@ -151,7 +151,7 @@ public class ExampleActor extends RaftActor implements RaftActorRecoveryCohort,
         state.putAll(((MapState)snapshotState).state);
 
         if (LOG.isDebugEnabled()) {
         state.putAll(((MapState)snapshotState).state);
 
         if (LOG.isDebugEnabled()) {
-            LOG.debug("Snapshot applied to state : {}", ((HashMap<?, ?>) state).size());
+            LOG.debug("Snapshot applied to state : {}", state.size());
         }
     }
 
         }
     }
 
index 0d11d7201d7cbd18efa2909128c5ea2b1d3482a9..f457074a7b05d712892ba2f1b882a7a0fc9921db 100644 (file)
@@ -88,13 +88,11 @@ public class ExampleRoleChangeListener extends AbstractUntypedActor implements A
     }
 
     private void populateRegistry(String memberName) {
     }
 
     private void populateRegistry(String memberName) {
-        for (String shard: SHARDS_TO_MONITOR) {
-            String notifier = new StringBuilder().append(NOTIFIER_AKKA_URL).append(memberName)
+        String notifier = new StringBuilder().append(NOTIFIER_AKKA_URL).append(memberName)
                 .append("/").append(memberName).append("-notifier").toString();
 
                 .append("/").append(memberName).append("-notifier").toString();
 
-            if (!notifierRegistrationStatus.containsKey(notifier)) {
-                notifierRegistrationStatus.put(notifier, false);
-            }
+        if (!notifierRegistrationStatus.containsKey(notifier)) {
+            notifierRegistrationStatus.put(notifier, false);
         }
 
         if (!registrationSchedule.isCancelled()) {
         }
 
         if (!registrationSchedule.isCancelled()) {
index a3fe498e8dbfdb662ed431ff4a2facab99fb6e94..96712eef4159542dd30726a2e9389f26821a5cb9 100644 (file)
@@ -31,7 +31,7 @@ public class LogGenerator {
         clientToLoggingThread.remove(client);
     }
 
         clientToLoggingThread.remove(client);
     }
 
-    public class LoggingThread implements Runnable {
+    public static class LoggingThread implements Runnable {
 
         private final ActorRef clientActor;
         private volatile boolean stopLogging = false;
 
         private final ActorRef clientActor;
         private volatile boolean stopLogging = false;
index 5c3b49f247214733179641c59e61887e587ebaba..54ff92993a9ad23e2236b26c2944a28c944f9ba3 100644 (file)
@@ -14,6 +14,7 @@ import akka.actor.PoisonPill;
 import com.google.common.base.Optional;
 import java.io.BufferedReader;
 import java.io.InputStreamReader;
 import com.google.common.base.Optional;
 import java.io.BufferedReader;
 import java.io.InputStreamReader;
+import java.nio.charset.Charset;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
@@ -54,7 +55,7 @@ public final class Main {
 
         ActorRef clientActor = ACTOR_SYSTEM.actorOf(ClientActor.props(example1Actor));
         BufferedReader br =
 
         ActorRef clientActor = ACTOR_SYSTEM.actorOf(ClientActor.props(example1Actor));
         BufferedReader br =
-            new BufferedReader(new InputStreamReader(System.in));
+            new BufferedReader(new InputStreamReader(System.in, Charset.defaultCharset()));
 
         System.out.println("Usage :");
         System.out.println("s <1-3> to start a peer");
 
         System.out.println("Usage :");
         System.out.println("s <1-3> to start a peer");
@@ -64,6 +65,9 @@ public final class Main {
             System.out.print("Enter command (0 to exit):");
             try {
                 String line = br.readLine();
             System.out.print("Enter command (0 to exit):");
             try {
                 String line = br.readLine();
+                if (line == null) {
+                    continue;
+                }
                 String[] split = line.split(" ");
                 if (split.length > 1) {
                     String command = split[0];
                 String[] split = line.split(" ");
                 if (split.length > 1) {
                     String command = split[0];
index ab980f77835d038b8d127fce286343416be58bc0..8f414c7a6b2dd636818463fb29d8280b858835df 100644 (file)
@@ -15,6 +15,7 @@ import com.google.common.collect.Lists;
 import com.typesafe.config.ConfigFactory;
 import java.io.BufferedReader;
 import java.io.InputStreamReader;
 import com.typesafe.config.ConfigFactory;
 import java.io.BufferedReader;
 import java.io.InputStreamReader;
+import java.nio.charset.Charset;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -75,9 +76,12 @@ public class TestDriver {
         System.out.println("Enter command (type bye to exit):");
 
 
         System.out.println("Enter command (type bye to exit):");
 
 
-        BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
+        BufferedReader br = new BufferedReader(new InputStreamReader(System.in, Charset.defaultCharset()));
         while (true) {
             String command = br.readLine();
         while (true) {
             String command = br.readLine();
+            if (command == null) {
+                continue;
+            }
             if (command.startsWith("bye")) {
                 System.exit(0);
 
             if (command.startsWith("bye")) {
                 System.exit(0);
 
index 6caaa6d1f6fbb9d9a8ab253e0fa129a40c99299f..13bf461ade0acf04e5d134813ae34d47ad66dfbf 100644 (file)
   <version>1.8.0-SNAPSHOT</version>
   <packaging>bundle</packaging>
 
   <version>1.8.0-SNAPSHOT</version>
   <packaging>bundle</packaging>
 
+  <properties>
+    <!-- The Java classes are mostly config yang-generated wjich we don't want analyzed by findbugs.
+          However the generated package names don't conform to the findbugs exclusion filter
+          hence we explicitly skip findbugs. -->
+    <findbugs.skip>true</findbugs.skip>
+  </properties>
+
   <dependencies>
     <dependency>
       <groupId>org.opendaylight.controller</groupId>
   <dependencies>
     <dependency>
       <groupId>org.opendaylight.controller</groupId>
index 55890c0edceb19782dc78f0c8914b30bad5f475c..f0ed08981989963345425f3b44997c6e6b01d4e8 100644 (file)
   <version>1.8.0-SNAPSHOT</version>
   <packaging>bundle</packaging>
 
   <version>1.8.0-SNAPSHOT</version>
   <packaging>bundle</packaging>
 
+  <properties>
+    <!-- The Java classes are mostly config yang-generated wjich we don't want analyzed by findbugs.
+         However the generated package names don't conform to the findbugs exclusion filter
+         hence we explicitly skip findbugs. -->
+    <findbugs.skip>true</findbugs.skip>
+  </properties>
+
   <dependencies>
     <dependency>
       <groupId>org.opendaylight.controller</groupId>
   <dependencies>
     <dependency>
       <groupId>org.opendaylight.controller</groupId>
index f2caddc718f46370f4b10d6e8a660777f1e2f35c..2959a2825a7f1d7d50c15f649d91d7395b72c5fc 100755 (executable)
@@ -33,8 +33,8 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public abstract class AbstractDOMDataBroker extends AbstractDOMForwardedTransactionFactory<DOMStore> implements
-        DOMDataBroker, AutoCloseable {
+public abstract class AbstractDOMDataBroker extends AbstractDOMForwardedTransactionFactory<DOMStore>
+       implements DOMDataBroker {
     private static final Logger LOG = LoggerFactory.getLogger(AbstractDOMDataBroker.class);
 
     private final AtomicLong txNum = new AtomicLong();
     private static final Logger LOG = LoggerFactory.getLogger(AbstractDOMDataBroker.class);
 
     private final AtomicLong txNum = new AtomicLong();
index bb2275f1fb508643225ffc00263f4747172444da..95d9e2171bcc562337987029d6720dd64dde9ab2 100644 (file)
@@ -157,7 +157,7 @@ public final class DOMNotificationRouter implements AutoCloseable, DOMNotificati
     private void notifyListenerTypesChanged(final Set<SchemaPath> typesAfter) {
         final List<ListenerRegistration<DOMNotificationSubscriptionListener>> listenersAfter = ImmutableList
                 .copyOf(subscriptionListeners.getListeners());
     private void notifyListenerTypesChanged(final Set<SchemaPath> typesAfter) {
         final List<ListenerRegistration<DOMNotificationSubscriptionListener>> listenersAfter = ImmutableList
                 .copyOf(subscriptionListeners.getListeners());
-        executor.submit(() -> {
+        executor.execute(() -> {
             for (final ListenerRegistration<DOMNotificationSubscriptionListener> subListener : listenersAfter) {
                 try {
                     subListener.getInstance().onSubscriptionChanged(typesAfter);
             for (final ListenerRegistration<DOMNotificationSubscriptionListener> subListener : listenersAfter) {
                 try {
                     subListener.getInstance().onSubscriptionChanged(typesAfter);
@@ -172,7 +172,7 @@ public final class DOMNotificationRouter implements AutoCloseable, DOMNotificati
     public <L extends DOMNotificationSubscriptionListener> ListenerRegistration<L> registerSubscriptionListener(
             final L listener) {
         final Set<SchemaPath> initialTypes = listeners.keySet();
     public <L extends DOMNotificationSubscriptionListener> ListenerRegistration<L> registerSubscriptionListener(
             final L listener) {
         final Set<SchemaPath> initialTypes = listeners.keySet();
-        executor.submit(() -> listener.onSubscriptionChanged(initialTypes));
+        executor.execute(() -> listener.onSubscriptionChanged(initialTypes));
         return subscriptionListeners.registerWithType(listener);
     }
 
         return subscriptionListeners.registerWithType(listener);
     }
 
index a1d5faee7ccd113953448db7da8001766c443692..8aa9f0d503eb1a80135cfd5b813ac22450434a24 100644 (file)
@@ -48,11 +48,9 @@ final class DOMNotificationRouterEvent {
         LOG.trace("Start delivery of notification {}", notification);
         for (ListenerRegistration<? extends DOMNotificationListener> r : subscribers) {
             final DOMNotificationListener listener = r.getInstance();
         LOG.trace("Start delivery of notification {}", notification);
         for (ListenerRegistration<? extends DOMNotificationListener> r : subscribers) {
             final DOMNotificationListener listener = r.getInstance();
-            if (listener != null) {
-                LOG.trace("Notifying listener {}", listener);
-                listener.onNotification(notification);
-                LOG.trace("Listener notification completed");
-            }
+            LOG.trace("Notifying listener {}", listener);
+            listener.onNotification(notification);
+            LOG.trace("Listener notification completed");
         }
         LOG.trace("Delivery completed");
     }
         }
         LOG.trace("Delivery completed");
     }
@@ -63,4 +61,4 @@ final class DOMNotificationRouterEvent {
         subscribers = null;
         future = null;
     }
         subscribers = null;
         future = null;
     }
-}
\ No newline at end of file
+}
index fd8f93b67dab930e71ee17ed64570f9639878a9a..63aba6e5f729267ac39340ab4e364bc8f8df6c1c 100644 (file)
@@ -22,10 +22,11 @@ final class PingPongFuture extends AbstractCheckedFuture<Void, TransactionCommit
 
     @Override
     protected TransactionCommitFailedException mapException(final Exception exception) {
 
     @Override
     protected TransactionCommitFailedException mapException(final Exception exception) {
-        if (exception.getCause() instanceof TransactionCommitFailedException) {
-            return (TransactionCommitFailedException) exception.getCause();
+        final Throwable cause = exception.getCause();
+        if (cause instanceof TransactionCommitFailedException) {
+            return (TransactionCommitFailedException) cause;
         } else {
         } else {
-            return new TransactionCommitFailedException(exception.getMessage(), exception.getCause());
+            return new TransactionCommitFailedException(exception.getMessage(), cause);
         }
     }
 }
         }
     }
 }
index bbff5dabbbdd37dadffb7dfd3d47a803b5fa62e2..8f04d09e821e678b3471c6974f1b490760a8c1cb 100644 (file)
@@ -7,6 +7,7 @@
  */
 package org.opendaylight.controller.md.sal.dom.broker.impl;
 
  */
 package org.opendaylight.controller.md.sal.dom.broker.impl;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
 import org.opendaylight.yangtools.util.concurrent.ExceptionMapper;
 
 import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
 import org.opendaylight.yangtools.util.concurrent.ExceptionMapper;
 
@@ -35,4 +36,10 @@ public final class TransactionCommitFailedExceptionMapper extends ExceptionMappe
     protected TransactionCommitFailedException newWithCause(final String message, final Throwable cause) {
         return new TransactionCommitFailedException(message, cause);
     }
     protected TransactionCommitFailedException newWithCause(final String message, final Throwable cause) {
         return new TransactionCommitFailedException(message, cause);
     }
-}
\ No newline at end of file
+
+    @Override
+    @SuppressFBWarnings("BC_UNCONFIRMED_CAST_OF_RETURN_VALUE")
+    public TransactionCommitFailedException apply(Exception input) {
+        return super.apply(input);
+    }
+}
index 84e19a34fc035bf373189fc60d52ff9da67fc9ce..3f9cdd6d314c693914d847303865467718bc4567 100644 (file)
@@ -24,7 +24,7 @@ import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.SettableFuture;
 import java.util.Map;
 import java.util.Queue;
 import com.google.common.util.concurrent.SettableFuture;
 import java.util.Map;
 import java.util.Queue;
-import javax.annotation.Nullable;
+import javax.annotation.Nonnull;
 import javax.annotation.concurrent.NotThreadSafe;
 import org.opendaylight.controller.md.sal.common.api.TransactionStatus;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import javax.annotation.concurrent.NotThreadSafe;
 import org.opendaylight.controller.md.sal.common.api.TransactionStatus;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
@@ -131,7 +131,7 @@ class ShardedDOMDataBrokerDelegatingReadWriteTransaction implements DOMDataReadW
         final Queue<Modification> currentHistory = Lists.newLinkedList(modificationHistoryMap.get(store));
         Futures.addCallback(initialReadMap.get(store), new FutureCallback<Optional<NormalizedNode<?, ?>>>() {
             @Override
         final Queue<Modification> currentHistory = Lists.newLinkedList(modificationHistoryMap.get(store));
         Futures.addCallback(initialReadMap.get(store), new FutureCallback<Optional<NormalizedNode<?, ?>>>() {
             @Override
-            public void onSuccess(@Nullable final Optional<NormalizedNode<?, ?>> result) {
+            public void onSuccess(@Nonnull final Optional<NormalizedNode<?, ?>> result) {
                 final DataTreeModification mod = snapshotMap.get(store).newModification();
                 if (result.isPresent()) {
                     mod.write(path, result.get());
                 final DataTreeModification mod = snapshotMap.get(store).newModification();
                 if (result.isPresent()) {
                     mod.write(path, result.get());
index 5d91a742a34787895032b0e123bf5db29eb169e4..c8a59f3419b002aa99f1c5bd8f2b3e4173dbbbf9 100644 (file)
@@ -9,6 +9,7 @@ package org.opendaylight.controller.md.sal.dom.clustering.impl;
 
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import javax.annotation.Nonnull;
 import org.opendaylight.controller.md.sal.common.api.clustering.CandidateAlreadyRegisteredException;
 import org.opendaylight.controller.md.sal.common.api.clustering.Entity;
 import javax.annotation.Nonnull;
 import org.opendaylight.controller.md.sal.common.api.clustering.CandidateAlreadyRegisteredException;
 import org.opendaylight.controller.md.sal.common.api.clustering.Entity;
@@ -135,9 +136,11 @@ public class LegacyEntityOwnershipServiceAdapter implements EntityOwnershipServi
         }
 
         @Override
         }
 
         @Override
+        @SuppressFBWarnings("BC_UNCONFIRMED_CAST_OF_RETURN_VALUE")
         public void ownershipChanged(final DOMEntityOwnershipChange ownershipChange) {
         public void ownershipChanged(final DOMEntityOwnershipChange ownershipChange) {
-            Entity entity = new Entity(ownershipChange.getEntity().getType(),
-                                       ownershipChange.getEntity().getIdentifier());
+            final DOMEntity domEntity = ownershipChange.getEntity();
+            Entity entity = new Entity(domEntity.getType(),
+                                       domEntity.getIdentifier());
             delegateListener.ownershipChanged(new EntityOwnershipChange(entity, ownershipChange.getState().wasOwner(),
                                                                         ownershipChange.getState().isOwner(),
                                                                         ownershipChange.getState().hasOwner(),
             delegateListener.ownershipChanged(new EntityOwnershipChange(entity, ownershipChange.getState().wasOwner(),
                                                                         ownershipChange.getState().isOwner(),
                                                                         ownershipChange.getState().hasOwner(),
index 00cc8dc51ea694e1c931f2c7ead8626531e5475f..a4f67df0f05f2514ac5a1938808b8c7788edb303 100644 (file)
@@ -12,7 +12,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.ClassToInstanceMap;
 import com.google.common.collect.MutableClassToInstanceMap;
 import java.util.Collection;
 import com.google.common.collect.ClassToInstanceMap;
 import com.google.common.collect.MutableClassToInstanceMap;
 import java.util.Collection;
-import javax.annotation.concurrent.GuardedBy;
+import java.util.concurrent.atomic.AtomicBoolean;
 import org.opendaylight.controller.sal.core.api.Broker.ConsumerSession;
 import org.opendaylight.controller.sal.core.api.BrokerService;
 import org.opendaylight.controller.sal.core.api.Consumer;
 import org.opendaylight.controller.sal.core.api.Broker.ConsumerSession;
 import org.opendaylight.controller.sal.core.api.BrokerService;
 import org.opendaylight.controller.sal.core.api.Consumer;
@@ -24,9 +24,8 @@ class ConsumerContextImpl implements ConsumerSession {
     private final ClassToInstanceMap<BrokerService> instantiatedServices = MutableClassToInstanceMap.create();
     private final Consumer consumer;
 
     private final ClassToInstanceMap<BrokerService> instantiatedServices = MutableClassToInstanceMap.create();
     private final Consumer consumer;
 
-    private BrokerImpl broker = null;
-    @GuardedBy("this")
-    private boolean closed = false;
+    private final BrokerImpl broker;
+    private final AtomicBoolean closed = new AtomicBoolean(false);
 
     ConsumerContextImpl(final Consumer provider, final BrokerImpl brokerImpl) {
         broker = brokerImpl;
 
     ConsumerContextImpl(final Consumer provider, final BrokerImpl brokerImpl) {
         broker = brokerImpl;
@@ -52,27 +51,21 @@ class ConsumerContextImpl implements ConsumerSession {
 
     @Override
     public void close() {
 
     @Override
     public void close() {
-        synchronized (this) {
-            if (closed) {
-                return;
+        if (closed.compareAndSet(false, true)) {
+            Collection<BrokerService> toStop = instantiatedServices.values();
+            for (BrokerService brokerService : toStop) {
+                if (brokerService instanceof AbstractBrokerServiceProxy<?>) {
+                    ((AbstractBrokerServiceProxy<?>) brokerService).close();
+                }
             }
             }
-            this.closed = true;
+            broker.consumerSessionClosed(this);
         }
         }
-
-        Collection<BrokerService> toStop = instantiatedServices.values();
-        for (BrokerService brokerService : toStop) {
-            if (brokerService instanceof AbstractBrokerServiceProxy<?>) {
-                ((AbstractBrokerServiceProxy<?>) brokerService).close();
-            }
-        }
-        broker.consumerSessionClosed(this);
-        broker = null;
     }
 
 
     @Override
     }
 
 
     @Override
-    public synchronized boolean isClosed() {
-        return closed;
+    public boolean isClosed() {
+        return closed.get();
     }
 
     /**
     }
 
     /**
@@ -95,6 +88,6 @@ class ConsumerContextImpl implements ConsumerSession {
     }
 
     protected final void checkNotClosed() {
     }
 
     protected final void checkNotClosed() {
-        Preconditions.checkState(!closed, "Session is closed.");
+        Preconditions.checkState(!closed.get(), "Session is closed.");
     }
 }
     }
 }
index f922882f7f0b8fd3528825ce8d0923de3c129d34..1ca9f5da96ce7276ce17eb7a477b32d1f78d123e 100644 (file)
@@ -7,7 +7,6 @@
  */
 package org.opendaylight.controller.sal.dom.broker.osgi;
 
  */
 package org.opendaylight.controller.sal.dom.broker.osgi;
 
-import java.util.Arrays;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataBroker;
 import org.opendaylight.controller.md.sal.dom.api.DOMMountPointService;
 import org.opendaylight.controller.sal.core.api.BrokerService;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataBroker;
 import org.opendaylight.controller.md.sal.dom.api.DOMMountPointService;
 import org.opendaylight.controller.sal.core.api.BrokerService;
@@ -23,25 +22,28 @@ public final class ProxyFactory {
     public static <T extends BrokerService> T createProxy(final ServiceReference<T> serviceRef, final T service) {
 
         Object createProxyImpl = ProxyFactory.createProxyImpl(serviceRef, service);
     public static <T extends BrokerService> T createProxy(final ServiceReference<T> serviceRef, final T service) {
 
         Object createProxyImpl = ProxyFactory.createProxyImpl(serviceRef, service);
-        return ((T) createProxyImpl);
+        return (T) createProxyImpl;
     }
 
     private static Object createProxyImpl(final ServiceReference<?> ref, final DOMMountPointService service) {
 
     }
 
     private static Object createProxyImpl(final ServiceReference<?> ref, final DOMMountPointService service) {
 
-        return new DOMMountPointServiceProxy(((ServiceReference<DOMMountPointService>) ref), service);
+        return new DOMMountPointServiceProxy((ServiceReference<DOMMountPointService>) ref, service);
     }
 
     private static Object createProxyImpl(final ServiceReference<?> ref, final SchemaService service) {
 
     }
 
     private static Object createProxyImpl(final ServiceReference<?> ref, final SchemaService service) {
 
-        return new SchemaServiceProxy(((ServiceReference<SchemaService>) ref), service);
+        return new SchemaServiceProxy((ServiceReference<SchemaService>) ref, service);
     }
 
     private static DOMDataBrokerProxy createProxyImpl(final ServiceReference<?> ref, final DOMDataBroker service) {
 
     }
 
     private static DOMDataBrokerProxy createProxyImpl(final ServiceReference<?> ref, final DOMDataBroker service) {
 
-        return new DOMDataBrokerProxy(((ServiceReference<DOMDataBroker>) ref), service);
+        return new DOMDataBrokerProxy((ServiceReference<DOMDataBroker>) ref, service);
     }
 
     private static Object createProxyImpl(final ServiceReference<?> ref, final BrokerService service) {
     }
 
     private static Object createProxyImpl(final ServiceReference<?> ref, final BrokerService service) {
+        if (service == null) {
+            throw new IllegalArgumentException("service can't be null");
+        }
 
         if (service instanceof DOMDataBroker) {
             return createProxyImpl(ref, (DOMDataBroker) service);
 
         if (service instanceof DOMDataBroker) {
             return createProxyImpl(ref, (DOMDataBroker) service);
@@ -49,16 +51,13 @@ public final class ProxyFactory {
             return createProxyImpl(ref, (SchemaService) service);
         } else if (service instanceof DOMMountPointService) {
             return createProxyImpl(ref, (DOMMountPointService) service);
             return createProxyImpl(ref, (SchemaService) service);
         } else if (service instanceof DOMMountPointService) {
             return createProxyImpl(ref, (DOMMountPointService) service);
-        } else if (service != null) {
-            return createProxyImplFallback(ref, service);
-        } else {
-            throw new IllegalArgumentException(
-                    "Unhandled parameter types: " + Arrays.<Object>asList(ref, service).toString());
         }
         }
+
+        return createProxyImplFallback(ref, service);
     }
 
     private static Object createProxyImplFallback(final ServiceReference<?> reference, final BrokerService service) {
 
         return service;
     }
     }
 
     private static Object createProxyImplFallback(final ServiceReference<?> reference, final BrokerService service) {
 
         return service;
     }
-}
\ No newline at end of file
+}
diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/util/YangSchemaUtils.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/util/YangSchemaUtils.java
deleted file mode 100644 (file)
index 7aa67a1..0000000
+++ /dev/null
@@ -1,236 +0,0 @@
-/*
- * Copyright (c) 2014 Cisco Systems, Inc. and others.  All rights reserved.
- *
- * This program and the accompanying materials are made available under the
- * terms of the Eclipse Public License v1.0 which accompanies this distribution,
- * and is available at http://www.eclipse.org/legal/epl-v10.html
- */
-package org.opendaylight.controller.sal.dom.broker.util;
-
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
-
-import com.google.common.collect.Lists;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Optional;
-import java.util.Set;
-import org.opendaylight.yangtools.yang.common.QName;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
-import org.opendaylight.yangtools.yang.model.api.ActionDefinition;
-import org.opendaylight.yangtools.yang.model.api.AugmentationSchemaNode;
-import org.opendaylight.yangtools.yang.model.api.CaseSchemaNode;
-import org.opendaylight.yangtools.yang.model.api.ChoiceSchemaNode;
-import org.opendaylight.yangtools.yang.model.api.ContainerSchemaNode;
-import org.opendaylight.yangtools.yang.model.api.DataNodeContainer;
-import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
-import org.opendaylight.yangtools.yang.model.api.GroupingDefinition;
-import org.opendaylight.yangtools.yang.model.api.LeafListSchemaNode;
-import org.opendaylight.yangtools.yang.model.api.LeafSchemaNode;
-import org.opendaylight.yangtools.yang.model.api.MustDefinition;
-import org.opendaylight.yangtools.yang.model.api.NotificationDefinition;
-import org.opendaylight.yangtools.yang.model.api.RevisionAwareXPath;
-import org.opendaylight.yangtools.yang.model.api.SchemaContext;
-import org.opendaylight.yangtools.yang.model.api.SchemaPath;
-import org.opendaylight.yangtools.yang.model.api.Status;
-import org.opendaylight.yangtools.yang.model.api.TypeDefinition;
-import org.opendaylight.yangtools.yang.model.api.UnknownSchemaNode;
-import org.opendaylight.yangtools.yang.model.api.UsesNode;
-
-public final class YangSchemaUtils {
-    private YangSchemaUtils() {
-        throw new UnsupportedOperationException("Utility class.");
-    }
-
-    public static DataSchemaNode getSchemaNode(final SchemaContext schema, final YangInstanceIdentifier path) {
-        checkArgument(schema != null, "YANG Schema must not be null.");
-        checkArgument(path != null, "Path must not be null.");
-        return getSchemaNode(schema, Lists.transform(path.getPathArguments(), PathArgument::getNodeType));
-    }
-
-    public static DataSchemaNode getSchemaNode(final SchemaContext schema, final Iterable<QName> path) {
-        checkArgument(schema != null, "YANG Schema must not be null.");
-        checkArgument(path != null, "Path must not be null.");
-        if (!path.iterator().hasNext()) {
-            return toRootDataNode(schema);
-        }
-
-        QName firstNode = path.iterator().next();
-        DataNodeContainer previous = schema.findModule(firstNode.getModule()).orElse(null);
-        Iterator<QName> iterator = path.iterator();
-
-        while (iterator.hasNext()) {
-            checkArgument(previous != null, "Supplied path does not resolve into valid schema node.");
-            QName arg = iterator.next();
-            DataSchemaNode currentNode = previous.getDataChildByName(arg);
-            if (currentNode == null) {
-                currentNode = searchInChoices(previous, arg);
-            }
-            if (currentNode instanceof DataNodeContainer) {
-                previous = (DataNodeContainer) currentNode;
-            } else if (currentNode instanceof LeafSchemaNode || currentNode instanceof LeafListSchemaNode) {
-                checkArgument(!iterator.hasNext(), "Path nests inside leaf node, which is not allowed.");
-                return currentNode;
-            }
-            checkState(currentNode != null, "Current node should not be null for %s", path);
-        }
-        checkState(previous instanceof DataSchemaNode,
-            "Schema node for %s should be instance of DataSchemaNode. Found %s", path, previous);
-        return (DataSchemaNode) previous;
-    }
-
-    private static DataSchemaNode searchInChoices(final DataNodeContainer node, final QName arg) {
-        for (DataSchemaNode child : node.getChildNodes()) {
-            if (child instanceof ChoiceSchemaNode) {
-                ChoiceSchemaNode choiceNode = (ChoiceSchemaNode) child;
-                DataSchemaNode potential = searchInCases(choiceNode, arg);
-                if (potential != null) {
-                    return potential;
-                }
-            }
-        }
-        return null;
-    }
-
-    private static DataSchemaNode searchInCases(final ChoiceSchemaNode choiceNode, final QName arg) {
-        for (CaseSchemaNode caseNode : choiceNode.getCases().values()) {
-            DataSchemaNode node = caseNode.getDataChildByName(arg);
-            if (node != null) {
-                return node;
-            }
-        }
-        return null;
-    }
-
-    private static ContainerSchemaNode toRootDataNode(final SchemaContext schema) {
-        return new NetconfDataRootNode(schema);
-    }
-
-    private static final class NetconfDataRootNode implements ContainerSchemaNode {
-
-        NetconfDataRootNode(final SchemaContext schema) {
-            // TODO Auto-generated constructor stub
-        }
-
-        @Override
-        public Set<TypeDefinition<?>> getTypeDefinitions() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public Set<DataSchemaNode> getChildNodes() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public Set<GroupingDefinition> getGroupings() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public Optional<DataSchemaNode> findDataChildByName(final QName name) {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public Set<UsesNode> getUses() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public Set<AugmentationSchemaNode> getAvailableAugmentations() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public boolean isAugmenting() {
-            // TODO Auto-generated method stub
-            return false;
-        }
-
-        @Override
-        public boolean isAddedByUses() {
-            // TODO Auto-generated method stub
-            return false;
-        }
-
-        @Override
-        public boolean isConfiguration() {
-            // TODO Auto-generated method stub
-            return false;
-        }
-
-        @Override
-        public QName getQName() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public SchemaPath getPath() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public Optional<String> getDescription() {
-            return Optional.empty();
-        }
-
-        @Override
-        public Optional<String> getReference() {
-            return Optional.empty();
-        }
-
-        @Override
-        public Status getStatus() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public List<UnknownSchemaNode> getUnknownSchemaNodes() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public boolean isPresenceContainer() {
-            // TODO Auto-generated method stub
-            return false;
-        }
-
-        @Override
-        public Set<NotificationDefinition> getNotifications() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public Set<ActionDefinition> getActions() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public Optional<RevisionAwareXPath> getWhenCondition() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-
-        @Override
-        public Collection<MustDefinition> getMustConstraints() {
-            // TODO Auto-generated method stub
-            return null;
-        }
-    }
-
-}
index 33ec2ed7d6dc9fdb4991a006ec6b993347f6d6d1..857a61b18cb896e23e00dde7fca6798322f1ce3c 100644 (file)
@@ -96,7 +96,7 @@ public class CarProvider implements CarService {
 
     public void close() {
         stopThread();
 
     public void close() {
         stopThread();
-        unregisterCommitCohort();
+        closeCommitCohortRegistration();
     }
 
     private void stopThread() {
     }
 
     private void stopThread() {
@@ -254,11 +254,8 @@ public class CarProvider implements CarService {
         final ListenerRegistration<CarDataTreeChangeListener> carsDtclRegistration =
                 dataProvider.registerDataTreeChangeListener(CARS_DTID, new CarDataTreeChangeListener());
 
         final ListenerRegistration<CarDataTreeChangeListener> carsDtclRegistration =
                 dataProvider.registerDataTreeChangeListener(CARS_DTID, new CarDataTreeChangeListener());
 
-        if (carsDtclRegistration != null) {
-            carsDtclRegistrations.add(carsDtclRegistration);
-            return RpcResultBuilder.<Void>success().buildFuture();
-        }
-        return RpcResultBuilder.<Void>failed().buildFuture();
+        carsDtclRegistrations.add(carsDtclRegistration);
+        return RpcResultBuilder.<Void>success().buildFuture();
     }
 
     @Override
     }
 
     @Override
@@ -279,18 +276,17 @@ public class CarProvider implements CarService {
     @Override
     @SuppressWarnings("checkstyle:IllegalCatch")
     public Future<RpcResult<Void>> unregisterCommitCohort() {
     @Override
     @SuppressWarnings("checkstyle:IllegalCatch")
     public Future<RpcResult<Void>> unregisterCommitCohort() {
+        closeCommitCohortRegistration();
+
+        return RpcResultBuilder.<Void>success().buildFuture();
+    }
+
+    private void closeCommitCohortRegistration() {
         final DOMDataTreeCommitCohortRegistration<CarEntryDataTreeCommitCohort> reg = commitCohortReg.getAndSet(null);
         if (reg != null) {
         final DOMDataTreeCommitCohortRegistration<CarEntryDataTreeCommitCohort> reg = commitCohortReg.getAndSet(null);
         if (reg != null) {
-            try {
-                reg.close();
-                LOG_CAR_PROVIDER.info("Unregistered commit cohort");
-            } catch (Exception e) {
-                return RpcResultBuilder.<Void>failed().withError(ErrorType.APPLICATION,
-                        "Error closing commit cohort registration", e).buildFuture();
-            }
+            reg.close();
+            LOG_CAR_PROVIDER.info("Unregistered commit cohort");
         }
         }
-
-        return RpcResultBuilder.<Void>success().buildFuture();
     }
 
     @Override
     }
 
     @Override
index c5427bef595e355983658c2946c6fbb5953951be..ec3da0ef36958e8684470f9b7da3619fab4bbeb8 100644 (file)
@@ -21,8 +21,6 @@ import com.google.common.base.Strings;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.SettableFuture;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.SettableFuture;
-import java.io.PrintWriter;
-import java.io.StringWriter;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -552,10 +550,8 @@ public class MdsalLowLevelTestProvider implements OdlMdsalLowlevelControlService
                 new CheckPublishNotificationsOutputBuilder().setActive(!task.isFinished());
 
         if (task.getLastError() != null) {
                 new CheckPublishNotificationsOutputBuilder().setActive(!task.isFinished());
 
         if (task.getLastError() != null) {
-            final StringWriter sw = new StringWriter();
-            final PrintWriter pw = new PrintWriter(sw);
             LOG.error("Last error for {}", task, task.getLastError());
             LOG.error("Last error for {}", task, task.getLastError());
-            checkPublishNotificationsOutputBuilder.setLastError(task.getLastError().toString() + sw.toString());
+            checkPublishNotificationsOutputBuilder.setLastError(task.getLastError().toString());
         }
 
         final CheckPublishNotificationsOutput output =
         }
 
         final CheckPublishNotificationsOutput output =
index 758185346c896311dea2f1c4df11243fb5efef4f..f0873034b4e44be30398a490021607fbf1385fdf 100644 (file)
@@ -32,6 +32,7 @@ final class FinalizableScheduledExecutorService extends ScheduledThreadPoolExecu
     @Override
     @SuppressWarnings("checkstyle:NoFinalizer")
     protected void finalize() {
     @Override
     @SuppressWarnings("checkstyle:NoFinalizer")
     protected void finalize() {
+        super.finalize();
         super.shutdownNow();
     }
 }
         super.shutdownNow();
     }
 }
index 2894ee35986e95ac102972efb926e341d0f8d90c..3ca46b128fdacf6dea475d47d2d6c5f4ec554fc5 100644 (file)
@@ -13,6 +13,7 @@ import com.google.common.util.concurrent.ListenableFuture;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonService;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceRegistration;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonService;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceRegistration;
@@ -32,7 +33,7 @@ public class FlappingSingletonService implements ClusterSingletonService {
     private final ClusterSingletonServiceProvider singletonServiceProvider;
     private final AtomicBoolean active = new AtomicBoolean(true);
 
     private final ClusterSingletonServiceProvider singletonServiceProvider;
     private final AtomicBoolean active = new AtomicBoolean(true);
 
-    private volatile long flapCount = 0;
+    private final AtomicLong flapCount = new AtomicLong();
     private volatile ClusterSingletonServiceRegistration registration;
 
     public FlappingSingletonService(final ClusterSingletonServiceProvider singletonServiceProvider) {
     private volatile ClusterSingletonServiceRegistration registration;
 
     public FlappingSingletonService(final ClusterSingletonServiceProvider singletonServiceProvider) {
@@ -56,7 +57,9 @@ public class FlappingSingletonService implements ClusterSingletonService {
             } catch (Exception e) {
                 LOG.warn("There was a problem closing flapping singleton service.", e);
                 setInactive();
             } catch (Exception e) {
                 LOG.warn("There was a problem closing flapping singleton service.", e);
                 setInactive();
-                flapCount = -flapCount;
+
+                final long count = flapCount.get();
+                flapCount.compareAndSet(count, -count);
             }
         });
     }
             }
         });
     }
@@ -66,7 +69,7 @@ public class FlappingSingletonService implements ClusterSingletonService {
     public ListenableFuture<Void> closeServiceInstance() {
         LOG.debug("Closing flapping-singleton-service, flapCount: {}", flapCount);
 
     public ListenableFuture<Void> closeServiceInstance() {
         LOG.debug("Closing flapping-singleton-service, flapCount: {}", flapCount);
 
-        flapCount++;
+        flapCount.incrementAndGet();
         if (active.get()) {
             // TODO direct registration/close seem to trigger a bug in singleton state transitions,
             // remove  whole executor shenanigans after it's fixed.
         if (active.get()) {
             // TODO direct registration/close seem to trigger a bug in singleton state transitions,
             // remove  whole executor shenanigans after it's fixed.
@@ -78,7 +81,9 @@ public class FlappingSingletonService implements ClusterSingletonService {
                 } catch (RuntimeException e) {
                     LOG.warn("There was a problem re-registering flapping singleton service.", e);
                     setInactive();
                 } catch (RuntimeException e) {
                     LOG.warn("There was a problem re-registering flapping singleton service.", e);
                     setInactive();
-                    flapCount = -flapCount - 1;
+
+                    final long count = flapCount.get();
+                    flapCount.compareAndSet(count, -count - 1);
                 }
 
             }, 200, TimeUnit.MILLISECONDS);
                 }
 
             }, 200, TimeUnit.MILLISECONDS);
@@ -96,6 +101,6 @@ public class FlappingSingletonService implements ClusterSingletonService {
         LOG.debug("Setting flapping-singleton-service to inactive, flap-count: {}", flapCount);
 
         active.set(false);
         LOG.debug("Setting flapping-singleton-service to inactive, flap-count: {}", flapCount);
 
         active.set(false);
-        return flapCount;
+        return flapCount.get();
     }
 }
     }
 }