QPID-6142: Fix NPE thrown on startup when broker fails to start due to CO validation exceptions

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1631732 13f79535-47bb-0310-9956-ffa450edef68

8 files changed, 76 insertions, 17 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/Broker.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/Broker.java
index 0b925d130c..7780f060bb 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/Broker.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/Broker.java
@@ -58,6 +58,17 @@ public class Broker implements BrokerShutdownProvider
     private EventLogger _eventLogger;
     private boolean _configuringOwnLogging = false;
     private final TaskExecutor _taskExecutor = new TaskExecutorImpl();
+    private final boolean _exitJVMOnShutdownWithNonZeroExitCode;
+
+    public Broker()
+    {
+        this(false);
+    }
+
+    public Broker(boolean exitJVMOnShutdownWithNonZeroExitCode)
+    {
+        this._exitJVMOnShutdownWithNonZeroExitCode = exitJVMOnShutdownWithNonZeroExitCode;
+    }
 
     protected static class InitException extends RuntimeException
     {
@@ -71,6 +82,12 @@ public class Broker implements BrokerShutdownProvider
 
     public void shutdown()
     {
+        shutdown(0);
+    }
+
+    @Override
+    public void shutdown(int exitStatusCode)
+    {
         try
         {
             removeShutdownHook();
@@ -92,6 +109,11 @@ public class Broker implements BrokerShutdownProvider
                 {
                     LogManager.shutdown();
                 }
+
+                if (_exitJVMOnShutdownWithNonZeroExitCode && exitStatusCode != 0)
+                {
+                    System.exit(exitStatusCode);
+                }
             }
         }
     }
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/BrokerMessages.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/BrokerMessages.java
index 064c9788b2..cd21f533a4 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/BrokerMessages.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/BrokerMessages.java
@@ -57,6 +57,7 @@ public class BrokerMessages
     public static final String SHUTTING_DOWN_LOG_HIERARCHY = DEFAULT_LOG_HIERARCHY_PREFIX + "broker.shutting_down";
     public static final String MANAGEMENT_MODE_LOG_HIERARCHY = DEFAULT_LOG_HIERARCHY_PREFIX + "broker.management_mode";
     public static final String STARTUP_LOG_HIERARCHY = DEFAULT_LOG_HIERARCHY_PREFIX + "broker.startup";
+    public static final String FATAL_ERROR_LOG_HIERARCHY = DEFAULT_LOG_HIERARCHY_PREFIX + "broker.fatal_error";
     public static final String READY_LOG_HIERARCHY = DEFAULT_LOG_HIERARCHY_PREFIX + "broker.ready";
 
     static
@@ -75,6 +76,7 @@ public class BrokerMessages
         Logger.getLogger(SHUTTING_DOWN_LOG_HIERARCHY);
         Logger.getLogger(MANAGEMENT_MODE_LOG_HIERARCHY);
         Logger.getLogger(STARTUP_LOG_HIERARCHY);
+        Logger.getLogger(FATAL_ERROR_LOG_HIERARCHY);
         Logger.getLogger(READY_LOG_HIERARCHY);
 
         _messages = ResourceBundle.getBundle("org.apache.qpid.server.logging.messages.Broker_logmessages", _currentLocale);
@@ -493,6 +495,38 @@ public class BrokerMessages
 
     /**
      * Log a Broker message of the Format:
+     * <pre>BRK-1016 : Fatal error : {0} : See log file for more information</pre>
+     * Optional values are contained in [square brackets] and are numbered
+     * sequentially in the method call.
+     *
+     */
+    public static LogMessage FATAL_ERROR(String param1)
+    {
+        String rawMessage = _messages.getString("FATAL_ERROR");
+
+        final Object[] messageArguments = {param1};
+        // Create a new MessageFormat to ensure thread safety.
+        // Sharing a MessageFormat and using applyPattern is not thread safe
+        MessageFormat formatter = new MessageFormat(rawMessage, _currentLocale);
+
+        final String message = formatter.format(messageArguments);
+
+        return new LogMessage()
+        {
+            public String toString()
+            {
+                return message;
+            }
+
+            public String getLogHierarchy()
+            {
+                return FATAL_ERROR_LOG_HIERARCHY;
+            }
+        };
+    }
+
+    /**
+     * Log a Broker message of the Format:
      * <pre>BRK-1004 : Qpid Broker Ready</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/Broker_logmessages.properties b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/Broker_logmessages.properties
index d764145ec8..322335a956 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/Broker_logmessages.properties
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/Broker_logmessages.properties
@@ -52,4 +52,6 @@ MANAGEMENT_MODE = BRK-1012 : Management Mode : User Details : {0} / {1}
 # 0 - Total message size
 # 1 - Target memory size
 FLOW_TO_DISK_ACTIVE = BRK-1014 : Message flow to disk active :  Message memory use {0,number,#}KB exceeds threshold {1,number,#.##}KB
-FLOW_TO_DISK_INACTIVE = BRK-1015 : Message flow to disk inactive : Message memory use {0,number,#}KB within threshold {1,number,#.##}KB
\ No newline at end of file
+FLOW_TO_DISK_INACTIVE = BRK-1015 : Message flow to disk inactive : Message memory use {0,number,#}KB within threshold {1,number,#.##}KB
+
+FATAL_ERROR = BRK-1016 : Fatal error : {0} : See log file for more information
\ No newline at end of file
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/BrokerShutdownProvider.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/BrokerShutdownProvider.java
index 5c8ab3e850..7eb0d3cc64 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/BrokerShutdownProvider.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/BrokerShutdownProvider.java
@@ -22,5 +22,5 @@ package org.apache.qpid.server.model;
 
 public interface BrokerShutdownProvider
 {
-    void shutdown();
+    void shutdown(int exitStatusCode);
 }
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
index ee008b1642..d413050cef 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
@@ -969,7 +969,8 @@ public class BrokerAdapter extends AbstractConfiguredObject<BrokerAdapter> imple
             BrokerShutdownProvider shutdownProvider = systemConfig.getBrokerShutdownProvider();
             if (shutdownProvider != null)
             {
-                shutdownProvider.shutdown();
+                _eventLogger.message(BrokerMessages.FATAL_ERROR(e.getMessage()));
+                shutdownProvider.shutdown(1);
             }
             else
             {
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java
index acfe7856c3..d32f0299d5 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java
@@ -32,6 +32,7 @@ import org.apache.qpid.server.logging.MessageLogger;
 import org.apache.qpid.server.logging.SystemOutMessageLogger;
 import org.apache.qpid.server.logging.messages.BrokerMessages;
 import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.model.SystemConfig;
 import org.apache.qpid.server.store.BrokerStoreUpgraderAndRecoverer;
 import org.apache.qpid.server.store.DurableConfigurationStore;
@@ -77,17 +78,17 @@ public class ApplicationRegistry implements IApplicationRegistry
         logStartupMessages(startupLogger);
 
         BrokerStoreUpgraderAndRecoverer upgrader = new BrokerStoreUpgraderAndRecoverer(_systemConfig);
-        _broker = upgrader.perform(_store);
+        Broker broker = upgrader.perform(_store);
+        _broker = broker;
 
-        _broker.setEventLogger(startupLogger);
-        _broker.open();
-
-        // starting the broker
-        //_broker.setDesiredState(State.ACTIVE);
-
-        startupLogger.message(BrokerMessages.READY());
-        _broker.setEventLogger(eventLogger);
+        broker.setEventLogger(startupLogger);
+        broker.open();
 
+        if (broker.getState() == State.ACTIVE)
+        {
+            startupLogger.message(BrokerMessages.READY());
+            broker.setEventLogger(eventLogger);
+        }
     }
 
     public void close()
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/store/BrokerRecovererTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/store/BrokerRecovererTest.java
index c220876a23..f4802481cb 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/store/BrokerRecovererTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/store/BrokerRecovererTest.java
@@ -34,7 +34,6 @@ import java.util.UUID;
 import junit.framework.TestCase;
 
 import org.apache.qpid.server.BrokerOptions;
-import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.configuration.updater.CurrentThreadTaskExecutor;
 import org.apache.qpid.server.configuration.updater.TaskExecutor;
 import org.apache.qpid.server.logging.EventLogger;
@@ -261,7 +260,7 @@ public class BrokerRecovererTest extends TestCase
             resolveObjects(_brokerEntry);
             Broker<?> broker = _systemConfig.getBroker();
             broker.open();
-            verify(_brokerShutdownProvider).shutdown();
+            verify(_brokerShutdownProvider).shutdown(1);
         }
     }
 
@@ -280,7 +279,7 @@ public class BrokerRecovererTest extends TestCase
 
         Broker<?> broker = (Broker<?>) recover.resolve();
         broker.open();
-        verify(_brokerShutdownProvider).shutdown();
+        verify(_brokerShutdownProvider).shutdown(1);
     }
 
     public void testIncorrectModelVersion() throws Exception
@@ -298,7 +297,7 @@ public class BrokerRecovererTest extends TestCase
                     _systemConfig.getObjectFactory().recover(_brokerEntry, _systemConfig);
             Broker<?> broker = (Broker<?>) recover.resolve();
             broker.open();
-            verify(_brokerShutdownProvider).shutdown();
+            verify(_brokerShutdownProvider).shutdown(1);
             reset(_brokerShutdownProvider);
         }
     }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/Main.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/Main.java
index d6b8a25b8a..76f8485884 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/Main.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/Main.java
@@ -454,7 +454,7 @@ public class Main
 
     protected void startBroker(final BrokerOptions options) throws Exception
     {
-        Broker broker = new Broker();
+        Broker broker = new Broker(true);
         broker.startup(options);
     }