Bug 8261: prevent TestBundleDiag from throwing NPE

There is a stack trace demonstrating this behavior in bug 8261.  In
short, TestBundleDiag is occasionally throwing NPE because a null
Bundle is returned from serviceReference.getBundle().  This change just
adds some null sanity checking in TestBundleDiag and
ServiceReferenceUtil.

Also, in ServiceReferenceUtil, a local variable is extracted just in
case the object is mutated between calls.  This at least gives a
consistent snapshot for usingBundles over the course of the method
call.

Change-Id: I334b0850be4a2045f9eeb260f5de4af542896dc1
Signed-off-by: Ryan Goulding <ryandgoulding@gmail.com>
(cherry picked from commit 667f362239f037fb01b670dbb20340ab2798ca1e)

diff --git a/bundles-test/src/main/java/org/opendaylight/odlparent/bundlestest/ServiceReferenceUtil.java b/bundles-test/src/main/java/org/opendaylight/odlparent/bundlestest/ServiceReferenceUtil.java
index 542e39a71b97040464292cd846ae298dddbff9cd..03db2a808165d31ffb6ee15dd03d9dec0f0d58d0 100644 (file)
--- a/bundles-test/src/main/java/org/opendaylight/odlparent/bundlestest/ServiceReferenceUtil.java
+++ b/bundles-test/src/main/java/org/opendaylight/odlparent/bundlestest/ServiceReferenceUtil.java
@@ -13,7 +13,10 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
+import org.osgi.framework.Bundle;
 import org.osgi.framework.ServiceReference;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Utilities for OSGi's {@link ServiceReference}.
@@ -22,25 +25,31 @@ import org.osgi.framework.ServiceReference;
  */
 public class ServiceReferenceUtil {
 
+    private static final Logger LOG = LoggerFactory.getLogger(ServiceReferenceUtil.class);
+
     public Map<String, Object> getProperties(ServiceReference<?> serviceRef) {
         String[] propertyKeys = serviceRef.getPropertyKeys();
         Map<String, Object> properties = new HashMap<>(propertyKeys.length);
         for (String propertyKey : propertyKeys) {
             Object propertyValue = serviceRef.getProperty(propertyKey);
-            if (propertyValue.getClass().isArray()) {
-                propertyValue = Arrays.asList((Object[]) propertyValue);
+            if (propertyValue != null) {
+                if (propertyValue.getClass().isArray()) {
+                    propertyValue = Arrays.asList((Object[]) propertyValue);
+                }
             }
+            // maintain the null value in the property map anyway
             properties.put(propertyKey, propertyValue);
         }
         return properties;
     }
 
     public List<String> getUsingBundleSymbolicNames(ServiceReference<?> serviceRef) {
-        if (serviceRef.getUsingBundles() == null) {
+        Bundle[] usingBundles = serviceRef.getUsingBundles();
+        if (usingBundles == null) {
             return Collections.emptyList();
         } else {
-            return Arrays.asList(serviceRef.getUsingBundles()).stream()
-                .map(bundle -> bundle.getSymbolicName()).collect(Collectors.toList());
+            return Arrays.asList(usingBundles).stream()
+                    .map(bundle -> bundle.getSymbolicName()).collect(Collectors.toList());
         }
     }
 

diff --git a/bundles-test/src/main/java/org/opendaylight/odlparent/bundlestest/TestBundleDiag.java b/bundles-test/src/main/java/org/opendaylight/odlparent/bundlestest/TestBundleDiag.java
index 1f7b52d8d8d29eb0471a6b32871e8c17bb72d160..2ee3b2689559c1560ec2c6addc5e92e5eaf1d656 100644 (file)
--- a/bundles-test/src/main/java/org/opendaylight/odlparent/bundlestest/TestBundleDiag.java
+++ b/bundles-test/src/main/java/org/opendaylight/odlparent/bundlestest/TestBundleDiag.java
@@ -15,6 +15,7 @@ import java.util.concurrent.TimeUnit;
 import org.apache.karaf.bundle.core.BundleService;
 import org.awaitility.Awaitility;
 import org.awaitility.core.ConditionTimeoutException;
+import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.framework.ServiceReference;
@@ -112,8 +113,14 @@ public class TestBundleDiag {
                 + "diag failure BundleService reported bundle(s) which are not active");
         try {
             for (ServiceReference<?> serviceRef : bundleContext.getAllServiceReferences(null, null)) {
-                LOG.info("{} defines OSGi Service {} used by {}", serviceRef.getBundle().getSymbolicName(),
-                        util.getProperties(serviceRef), util.getUsingBundleSymbolicNames(serviceRef));
+                Bundle bundle = serviceRef.getBundle();
+                // serviceRef.getBundle() can return null if the bundle was destroyed
+                if (bundle != null) {
+                    LOG.info("{} defines OSGi Service {} used by {}", bundle.getSymbolicName(),
+                            util.getProperties(serviceRef), util.getUsingBundleSymbolicNames(serviceRef));
+                } else {
+                    LOG.trace("skipping reporting service reference as the underlying bundle is null");
+                }
             }
         } catch (InvalidSyntaxException e) {
             LOG.error("logOSGiServices() failed due to InvalidSyntaxException", e);

diff --git a/bundles-test/src/test/java/org/opendaylight/odlparent/bundlestest/ServiceReferenceUtilTest.java b/bundles-test/src/test/java/org/opendaylight/odlparent/bundlestest/ServiceReferenceUtilTest.java
index 2d32084a661d927cfcd41dbc5747048e861d4883..23a18fd705d41fe5b7118013d370b301fe4fb415 100644 (file)
--- a/bundles-test/src/test/java/org/opendaylight/odlparent/bundlestest/ServiceReferenceUtilTest.java
+++ b/bundles-test/src/test/java/org/opendaylight/odlparent/bundlestest/ServiceReferenceUtilTest.java
@@ -9,7 +9,10 @@ package org.opendaylight.odlparent.bundlestest;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import java.util.Arrays;
+import java.util.Map;
 import org.junit.Test;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.ServiceReference;
@@ -30,48 +33,53 @@ public class ServiceReferenceUtilTest {
     public void testGetProperties() {
         assertThat(new ServiceReferenceUtil().getProperties(getServiceReference())).containsExactly(
                 "property1", "value1",
-                "property2", Arrays.asList(new String[] { "value2.1", "value2.2" }));
+                "property2", Arrays.asList(new String[] { "value2.1", "value2.2" }),
+                "property3", null);
     }
 
-    private ServiceReference<?> getServiceReference() {
-        return new ServiceReference<Object>() {
-
-            @Override
-            public Object getProperty(String key) {
-                if ("property1".equals(key)) {
-                    return "value1";
-                } else if ("property2".equals(key)) {
-                    return new String[] { "value2.1", "value2.2" };
-                } else {
-                    return null;
-                }
-            }
-
-            @Override
-            public String[] getPropertyKeys() {
-                return new String[] { "property1", "property2"};
-            }
-
-            @Override
-            public Bundle getBundle() {
-                return null;
-            }
-
-            @Override
-            public Bundle[] getUsingBundles() {
-                return null;
-            }
-
-            @Override
-            public boolean isAssignableTo(Bundle bundle, String className) {
-                return false;
-            }
-
-            @Override
-            public int compareTo(Object reference) {
-                return 0;
-            }
-        };
+    private static ServiceReference<?> getServiceReference() {
+        return new TestServiceReference();
+    }
+
+    private static final class TestServiceReference implements ServiceReference<Object> {
+
+        private Map<String, Object> properties = Maps.newHashMap();
+
+        TestServiceReference() {
+            properties.put("property1", "value1");
+            properties.put("property2", Lists.newArrayList("value2.1", "value2.2"));
+            properties.put("property3", null);
+        }
+
+        @Override
+        public Object getProperty(String key) {
+            return properties.get(key);
+        }
+
+        @Override
+        public String[] getPropertyKeys() {
+            return properties.keySet().stream().toArray(String[]::new);
+        }
+
+        @Override
+        public Bundle getBundle() {
+            return null;
+        }
+
+        @Override
+        public Bundle[] getUsingBundles() {
+            return null;
+        }
+
+        @Override
+        public boolean isAssignableTo(Bundle bundle, String className) {
+            return false;
+        }
+
+        @Override
+        public int compareTo(Object reference) {
+            return 0;
+        }
     }
 
 }

diff --git a/bundles4-test/src/main/java/org/opendaylight/odlparent/bundles4test/ServiceReferenceUtil.java b/bundles4-test/src/main/java/org/opendaylight/odlparent/bundles4test/ServiceReferenceUtil.java
index 41f9c2ca1608b2816754014aeaa690692e4161a1..767c7ebb51d88c727214a7154accedd7c4b672ed 100644 (file)
--- a/bundles4-test/src/main/java/org/opendaylight/odlparent/bundles4test/ServiceReferenceUtil.java
+++ b/bundles4-test/src/main/java/org/opendaylight/odlparent/bundles4test/ServiceReferenceUtil.java
@@ -13,7 +13,10 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
+import org.osgi.framework.Bundle;
 import org.osgi.framework.ServiceReference;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Utilities for OSGi's {@link ServiceReference}.
@@ -22,24 +25,30 @@ import org.osgi.framework.ServiceReference;
  */
 public class ServiceReferenceUtil {
 
+    private static final Logger LOG = LoggerFactory.getLogger(ServiceReferenceUtil.class);
+
     public Map<String, Object> getProperties(ServiceReference<?> serviceRef) {
         String[] propertyKeys = serviceRef.getPropertyKeys();
         Map<String, Object> properties = new HashMap<>(propertyKeys.length);
         for (String propertyKey : propertyKeys) {
             Object propertyValue = serviceRef.getProperty(propertyKey);
-            if (propertyValue.getClass().isArray()) {
-                propertyValue = Arrays.asList((Object[]) propertyValue);
+            if (propertyValue != null) {
+                if (propertyValue.getClass().isArray()) {
+                    propertyValue = Arrays.asList((Object[]) propertyValue);
+                }
             }
+            // maintain the null value in the property map anyway
             properties.put(propertyKey, propertyValue);
         }
         return properties;
     }
 
     public List<String> getUsingBundleSymbolicNames(ServiceReference<?> serviceRef) {
-        if (serviceRef.getUsingBundles() == null) {
+        Bundle[] usingBundles = serviceRef.getUsingBundles();
+        if (usingBundles == null) {
             return Collections.emptyList();
         } else {
-            return Arrays.asList(serviceRef.getUsingBundles()).stream()
+            return Arrays.asList(usingBundles).stream()
                 .map(bundle -> bundle.getSymbolicName()).collect(Collectors.toList());
         }
     }

diff --git a/bundles4-test/src/main/java/org/opendaylight/odlparent/bundles4test/TestBundleDiag.java b/bundles4-test/src/main/java/org/opendaylight/odlparent/bundles4test/TestBundleDiag.java
index a6ec619e23daed571335c83267745b6d32b2285a..e86e52ad94c768680b16b8dac9cdd2968549fedf 100644 (file)
--- a/bundles4-test/src/main/java/org/opendaylight/odlparent/bundles4test/TestBundleDiag.java
+++ b/bundles4-test/src/main/java/org/opendaylight/odlparent/bundles4test/TestBundleDiag.java
@@ -14,6 +14,7 @@ import java.util.concurrent.TimeUnit;
 import org.apache.karaf.bundle.core.BundleService;
 import org.awaitility.Awaitility;
 import org.awaitility.core.ConditionTimeoutException;
+import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.framework.ServiceReference;
@@ -127,8 +128,14 @@ public class TestBundleDiag {
                 + "diag failure BundleService reported bundle(s) which are not active");
         try {
             for (ServiceReference<?> serviceRef : bundleContext.getAllServiceReferences(null, null)) {
-                LOG.info("{} defines OSGi Service {} used by {}", serviceRef.getBundle().getSymbolicName(),
-                        util.getProperties(serviceRef), util.getUsingBundleSymbolicNames(serviceRef));
+                Bundle bundle = serviceRef.getBundle();
+                // serviceRef.getBundle() can return null if the bundle was destroyed
+                if (bundle != null) {
+                    LOG.info("{} defines OSGi Service {} used by {}", bundle.getSymbolicName(),
+                            util.getProperties(serviceRef), util.getUsingBundleSymbolicNames(serviceRef));
+                } else {
+                    LOG.trace("skipping reporting service reference as the underlying bundle is null");
+                }
             }
         } catch (InvalidSyntaxException e) {
             LOG.error("logOSGiServices() failed due to InvalidSyntaxException", e);

diff --git a/bundles4-test/src/test/java/org/opendaylight/odlparent/bundles4test/ServiceReferenceUtilTest.java b/bundles4-test/src/test/java/org/opendaylight/odlparent/bundles4test/ServiceReferenceUtilTest.java
index c3892ef4d59018eae123ef1e5406394c2e41d66a..99dbdff0881eb61b4bcc8c8f1ede55fbe4eb3906 100644 (file)
--- a/bundles4-test/src/test/java/org/opendaylight/odlparent/bundles4test/ServiceReferenceUtilTest.java
+++ b/bundles4-test/src/test/java/org/opendaylight/odlparent/bundles4test/ServiceReferenceUtilTest.java
@@ -9,7 +9,10 @@ package org.opendaylight.odlparent.bundles4test;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import java.util.Arrays;
+import java.util.Map;
 import org.junit.Test;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.ServiceReference;
@@ -30,48 +33,53 @@ public class ServiceReferenceUtilTest {
     public void testGetProperties() {
         assertThat(new ServiceReferenceUtil().getProperties(getServiceReference())).containsExactly(
                 "property1", "value1",
-                "property2", Arrays.asList(new String[] { "value2.1", "value2.2" }));
+                "property2", Arrays.asList(new String[] { "value2.1", "value2.2" }),
+                "property3", null);
     }
 
-    private ServiceReference<?> getServiceReference() {
-        return new ServiceReference<Object>() {
-
-            @Override
-            public Object getProperty(String key) {
-                if ("property1".equals(key)) {
-                    return "value1";
-                } else if ("property2".equals(key)) {
-                    return new String[] { "value2.1", "value2.2" };
-                } else {
-                    return null;
-                }
-            }
-
-            @Override
-            public String[] getPropertyKeys() {
-                return new String[] { "property1", "property2"};
-            }
-
-            @Override
-            public Bundle getBundle() {
-                return null;
-            }
-
-            @Override
-            public Bundle[] getUsingBundles() {
-                return null;
-            }
-
-            @Override
-            public boolean isAssignableTo(Bundle bundle, String className) {
-                return false;
-            }
-
-            @Override
-            public int compareTo(Object reference) {
-                return 0;
-            }
-        };
+    private static ServiceReference<?> getServiceReference() {
+        return new TestServiceReference();
+    }
+
+    private static final class TestServiceReference implements ServiceReference<Object> {
+
+        private Map<String, Object> properties = Maps.newHashMap();
+
+        TestServiceReference() {
+            properties.put("property1", "value1");
+            properties.put("property2", Lists.newArrayList("value2.1", "value2.2"));
+            properties.put("property3", null);
+        }
+
+        @Override
+        public Object getProperty(String key) {
+            return properties.get(key);
+        }
+
+        @Override
+        public String[] getPropertyKeys() {
+            return properties.keySet().stream().toArray(String[]::new);
+        }
+
+        @Override
+        public Bundle getBundle() {
+            return null;
+        }
+
+        @Override
+        public Bundle[] getUsingBundles() {
+            return null;
+        }
+
+        @Override
+        public boolean isAssignableTo(Bundle bundle, String className) {
+            return false;
+        }
+
+        @Override
+        public int compareTo(Object reference) {
+            return 0;
+        }
     }
 
 }