Bug 3754 - NPE at org.opendaylight.openflowplugin.openflow.md.core.ConnectionConducto...

Change-Id: Ib340a463076f805b29363c008bbdd214f25c2b78
Signed-off-by: Martin Bobak <mbobak@cisco.com>

diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/md/core/session/SessionContext.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/md/core/session/SessionContext.java
index eec52194be693942f4898e8ae575c0129e2999cb..221055b4aa847cc592901b25dda28d2b24f3efba 100644 (file)
--- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/md/core/session/SessionContext.java
+++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/md/core/session/SessionContext.java
@@ -12,10 +12,9 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
-
+import org.opendaylight.openflowplugin.api.openflow.md.ModelDrivenSwitch;
 import org.opendaylight.openflowplugin.api.openflow.md.core.ConnectionConductor;
 import org.opendaylight.openflowplugin.api.openflow.md.core.NotificationEnqueuer;
-import org.opendaylight.openflowplugin.api.openflow.md.ModelDrivenSwitch;
 import org.opendaylight.openflowplugin.api.openflow.md.core.NotificationQueueWrapper;
 import org.opendaylight.openflowplugin.api.openflow.md.core.SwitchConnectionDistinguisher;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.ControllerRole;
@@ -39,8 +38,7 @@ public interface SessionContext {
     GetFeaturesOutput getFeatures();
 
     /**
-     * @param auxiliaryKey
-     *            key under which the auxiliary conductor is stored
+     * @param auxiliaryKey key under which the auxiliary conductor is stored
      * @return list of auxiliary connection wrappers
      */
     ConnectionConductor getAuxiliaryConductor(
@@ -58,7 +56,7 @@ public interface SessionContext {
      * @param conductor
      */
     void addAuxiliaryConductor(SwitchConnectionDistinguisher auxiliaryKey,
-            ConnectionConductor conductor);
+                               ConnectionConductor conductor);
 
     /**
      * @param connectionCookie
@@ -84,24 +82,31 @@ public interface SessionContext {
 
     /**
      * Returns a map containing all OFPhysicalPorts of this switch.
+     *
      * @return The Map of OFPhysicalPort
      */
+    @Deprecated
     Map<Long, PortGrouping> getPhysicalPorts();
 
     /**
      * Returns a map containing all bandwidths for all OFPorts of this switch.
+     *
      * @return The Map of bandwidths for all OFPorts
      */
+    @Deprecated
     Map<Long, Boolean> getPortsBandwidth();
 
     /**
      * Returns a Set containing all port IDs of this switch.
+     *
      * @return The Set of port ID
      */
+    @Deprecated
     Set<Long> getPorts();
 
     /**
      * Returns OFPhysicalPort of the specified portNumber of this switch.
+     *
      * @param portNumber The port ID
      * @return OFPhysicalPort for the specified PortNumber
      */
@@ -109,6 +114,7 @@ public interface SessionContext {
 
     /**
      * Returns the bandwidth of the specified portNumber of this switch.
+     *
      * @param portNumber the port ID
      * @return bandwidth
      */
@@ -116,6 +122,7 @@ public interface SessionContext {
 
     /**
      * Returns True if the port is enabled,
+     *
      * @param portNumber
      * @return True if the port is enabled
      */
@@ -123,6 +130,7 @@ public interface SessionContext {
 
     /**
      * Returns True if the port is enabled.
+     *
      * @param port
      * @return True if the port is enabled
      */
@@ -130,6 +138,7 @@ public interface SessionContext {
 
     /**
      * Returns a list containing all enabled ports of this switch.
+     *
      * @return List containing all enabled ports of this switch
      */
     List<PortGrouping> getEnabledPorts();
@@ -137,15 +146,15 @@ public interface SessionContext {
     // TODO:: add listeners here, manager will set them and conductor use them
 
     /**
-     *  get message dispatch service to send the message to switch
+     * get message dispatch service to send the message to switch
      *
      * @return the message service
      */
     IMessageDispatchService getMessageDispatchService();
 
-   /**
-    * @return the unique xid for this session
-    */
+    /**
+     * @return the unique xid for this session
+     */
     Long getNextXid();
 
     /**

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/ConnectionConductorImpl.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/ConnectionConductorImpl.java
index 34c57c0cc6870b4fc0b1bafe88b6de94b9e49afd..5c4884716a7339b158d66fd595beac3af7e5c7aa 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/ConnectionConductorImpl.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/ConnectionConductorImpl.java
@@ -10,12 +10,10 @@ package org.opendaylight.openflowplugin.openflow.md.core;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.util.concurrent.Futures;
-
 import java.util.concurrent.Future;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
-
 import org.opendaylight.openflowjava.protocol.api.connection.ConnectionAdapter;
 import org.opendaylight.openflowjava.protocol.api.connection.ConnectionReadyListener;
 import org.opendaylight.openflowplugin.api.OFConstants;
@@ -124,11 +122,10 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
 
     /**
      * @param connectionAdapter
-     * @param ingressMaxQueueSize
-     *            ingress queue limit (blocking)
+     * @param ingressMaxQueueSize ingress queue limit (blocking)
      */
     public ConnectionConductorImpl(ConnectionAdapter connectionAdapter,
-            int ingressMaxQueueSize) {
+                                   int ingressMaxQueueSize) {
         this.connectionAdapter = connectionAdapter;
         this.ingressMaxQueueSize = ingressMaxQueueSize;
         conductorState = CONDUCTOR_STATE.HANDSHAKING;
@@ -147,7 +144,7 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
         hsPool = new ThreadPoolLoggingExecutor(handshakeThreadLimit,
                 handshakeThreadLimit, 0L, TimeUnit.MILLISECONDS,
                 new LinkedBlockingQueue<Runnable>(), "OFHandshake-"
-                        + conductorId);
+                + conductorId);
 
         connectionAdapter.setMessageListener(this);
         connectionAdapter.setSystemListener(this);
@@ -165,8 +162,7 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
     }
 
     /**
-     * @param errorHandler
-     *            the errorHandler to set
+     * @param errorHandler the errorHandler to set
      */
     @Override
     public void setErrorHandler(ErrorHandler errorHandler) {
@@ -210,8 +206,7 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
 
     /**
      * @param message
-     * @param queueType
-     *            enqueue type
+     * @param queueType enqueue type
      */
     private void enqueueMessage(OfHeader message, QueueType queueType) {
         queue.push(message, this, queueType);
@@ -275,8 +270,11 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
 
     @Override
     public void onPortStatusMessage(PortStatusMessage message) {
-        processPortStatusMsg(message);
-        enqueueMessage(message);
+        try {
+            processPortStatusMsg(message);
+        } finally {
+            enqueueMessage(message);
+        }
     }
 
     protected void processPortStatusMsg(PortStatus msg) {
@@ -298,9 +296,14 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
                     "can't get bandwidth info from port: {}, aborting port update",
                     msg.toString());
         } else {
-            this.getSessionContext().getPhysicalPorts().put(portNumber, msg);
-            this.getSessionContext().getPortsBandwidth()
-                    .put(portNumber, portBandwidth);
+            if (null != this.sessionContext) {
+                //FIXME these two properties are never used in code
+                this.getSessionContext().getPhysicalPorts().put(portNumber, msg);
+                this.getSessionContext().getPortsBandwidth()
+                        .put(portNumber, portBandwidth);
+            } else {
+                LOG.warn("Trying to process update port message before session context was created.");
+            }
         }
     }
 
@@ -368,8 +371,7 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
     }
 
     /**
-     * @param conductorState
-     *            the connectionState to set
+     * @param conductorState the connectionState to set
      */
     @Override
     public void setConductorState(CONDUCTOR_STATE conductorState) {
@@ -387,7 +389,7 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
     protected void checkState(CONDUCTOR_STATE expectedState) {
         if (!conductorState.equals(expectedState)) {
             LOG.warn("State of connection to switch {} is not correct, "
-                    + "terminating the connection",connectionAdapter.getRemoteAddress() );
+                    + "terminating the connection", connectionAdapter.getRemoteAddress());
             throw new IllegalStateException("Expected state: " + expectedState
                     + ", actual state:" + conductorState);
         }
@@ -486,7 +488,7 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
      * @param negotiatedVersion
      */
     protected void postHandshakeBasic(GetFeaturesOutput featureOutput,
-            Short negotiatedVersion) {
+                                      Short negotiatedVersion) {
         version = negotiatedVersion;
         if (version == OFConstants.OFP_VERSION_1_0) {
             // Because the GetFeaturesOutput contains information about the port
@@ -564,8 +566,7 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
     }
 
     /**
-     * @param isBitmapNegotiationEnable
-     *            the isBitmapNegotiationEnable to set
+     * @param isBitmapNegotiationEnable the isBitmapNegotiationEnable to set
      */
     public void setBitmapNegotiationEnable(boolean isBitmapNegotiationEnable) {
         this.isBitmapNegotiationEnable = isBitmapNegotiationEnable;
@@ -579,7 +580,7 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
     @Override
     public void close() {
         conductorState = CONDUCTOR_STATE.RIP;
-        if(handshakeContext != null){
+        if (handshakeContext != null) {
             try {
                 handshakeContext.close();
             } catch (Exception e) {
@@ -593,7 +594,7 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
     }
 
     private void shutdownPoolPolitely() {
-        LOG.debug("Terminating handshake pool for node {}",connectionAdapter.getRemoteAddress());
+        LOG.debug("Terminating handshake pool for node {}", connectionAdapter.getRemoteAddress());
         hsPool.shutdown();
         try {
             hsPool.awaitTermination(1, TimeUnit.SECONDS);
@@ -601,7 +602,7 @@ public class ConnectionConductorImpl implements OpenflowProtocolListener,
             LOG.debug("Error while awaiting termination of pool. Will force shutdown now.");
         } finally {
             hsPool.purge();
-            if (! hsPool.isTerminated()) {
+            if (!hsPool.isTerminated()) {
                 hsPool.shutdownNow();
             }
             LOG.debug("is handshake pool for node {} is terminated : {}",