Bug 7901: Prevent null Modification in BatchedModifications 83/52883/4
authorTom Pantelis <tpanteli@brocade.com>
Mon, 6 Mar 2017 13:44:41 +0000 (08:44 -0500)
committerTom Pantelis <tpanteli@brocade.com>
Thu, 9 Mar 2017 11:41:51 +0000 (11:41 +0000)
Caused by: java.lang.NullPointerException
   at org.opendaylight.controller.cluster.datastore.modification.
      MutableCompositeModification.writeExternal(MutableCompositeModification.java:120)

Somehow a null Modification instance got added to the modifications List.
Looking at the callers of addModification, it is not clear which call site
might have been the culprit. So I added a guard in addModification against
a null input Modification and logged an error with stack trace. I also
modified getModifications to return an immutable List to prevent callers
from directly modifying the List (there was one).

Change-Id: Ic63aa9daada0548da05fe663a0d22cdcb3e7bceb
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/RemoteTransactionContext.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardCommitCoordinator.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/modification/CompositeModification.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/modification/MutableCompositeModification.java

index 6334bdc..ba64e29 100644 (file)
@@ -139,13 +139,15 @@ public class RemoteTransactionContext extends AbstractTransactionContext {
             batchedModifications.setReady(ready);
             batchedModifications.setDoCommitOnReady(doCommitOnReady);
             batchedModifications.setTotalMessagesSent(++totalBatchedModificationsSent);
-            sent = executeOperationAsync(batchedModifications, actorContext.getTransactionCommitOperationTimeout());
 
+            final BatchedModifications toSend = batchedModifications;
             if (ready) {
                 batchedModifications = null;
             } else {
                 batchedModifications = newBatchedModifications();
             }
+
+            sent = executeOperationAsync(toSend, actorContext.getTransactionCommitOperationTimeout());
         }
 
         return sent;
index 9c65ed7..a695829 100644 (file)
@@ -266,7 +266,7 @@ class EntityOwnershipShardCommitCoordinator {
 
     private void newInflightCommitWithDifferentTransactionID() {
         BatchedModifications newBatchedModifications = newBatchedModifications();
-        newBatchedModifications.getModifications().addAll(inflightCommit.getModifications());
+        newBatchedModifications.addModifications(inflightCommit.getModifications());
         inflightCommit = newBatchedModifications;
     }
 
index 2bc6d1d..ced960d 100644 (file)
@@ -21,7 +21,7 @@ public interface CompositeModification extends Modification {
     /**
      * Get a list of modifications contained by this composite.
      *
-     * @return list of modifications
+     * @return an immutable list of modifications
      */
     List<Modification> getModifications();
 }
index 6372b21..f2a8c00 100644 (file)
@@ -13,6 +13,7 @@ import java.io.IOException;
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import org.opendaylight.controller.cluster.datastore.DataStoreVersions;
 import org.opendaylight.controller.cluster.datastore.messages.VersionedExternalizableMessage;
@@ -29,6 +30,7 @@ public class MutableCompositeModification extends VersionedExternalizableMessage
     private static final long serialVersionUID = 1L;
 
     private final List<Modification> modifications = new ArrayList<>();
+    private List<Modification> immutableModifications = null;
 
     public MutableCompositeModification() {
         this(DataStoreVersions.CURRENT_VERSION);
@@ -63,12 +65,23 @@ public class MutableCompositeModification extends VersionedExternalizableMessage
      * @param modification the modification to add.
      */
     public void addModification(Modification modification) {
+        Preconditions.checkNotNull(modification);
         modifications.add(modification);
     }
 
+    public void addModifications(Iterable<Modification> newMods) {
+        for (Modification mod : newMods) {
+            addModification(mod);
+        }
+    }
+
     @Override
     public List<Modification> getModifications() {
-        return modifications;
+        if (immutableModifications == null) {
+            immutableModifications = Collections.unmodifiableList(modifications);
+        }
+
+        return immutableModifications;
     }
 
     @Override

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.