QPID-5611 : Ensure the appropriate principals are available at the time of all event logging

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

23 files changed, 177 insertions, 134 deletions

diff --git a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/BDBHAMessageStore.java b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/BDBHAMessageStore.java
index d61da537f3..d99733acf0 100644
--- a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/BDBHAMessageStore.java
+++ b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/BDBHAMessageStore.java
@@ -619,12 +619,10 @@ public class BDBHAMessageStore extends AbstractBDBMessageStore implements HAMess
                 {
                     final String originalThreadName = Thread.currentThread().getName();
                     Thread.currentThread().setName(threadName);
-                    Subject subject = new Subject(false, SecurityManager.SYSTEM.getPrincipals(),
-                                                  Collections.emptySet(), Collections.emptySet());
-                    subject.getPrincipals().add(new TaskPrincipal("BDB HA State Change"));
+
                     try
                     {
-                        Subject.doAs(subject, new PrivilegedAction<Object>()
+                        Subject.doAs(SecurityManager.getSystemTaskSubject("BDB HA State Change"), new PrivilegedAction<Object>()
                         {
                             @Override
                             public Object run()
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 69d3950ce4..ffa1d65ec2 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
@@ -86,12 +86,9 @@ public class Broker
 
     public void startup(final BrokerOptions options) throws Exception
     {
-        Subject subject = SecurityManager.SYSTEM;
-        subject = new Subject(false, subject.getPrincipals(), subject.getPublicCredentials(), subject.getPrivateCredentials());
-        subject.getPrincipals().add(new TaskPrincipal("Broker"));
         _eventLogger = new EventLogger(new SystemOutMessageLogger());
 
-        Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+        Subject.doAs(SecurityManager.getSystemTaskSubject("Broker"), new PrivilegedExceptionAction<Object>()
         {
             @Override
             public Object run() throws Exception
@@ -282,11 +279,7 @@ public class Broker
     {
         public void run()
         {
-
-            Subject subject = SecurityManager.SYSTEM;
-            subject = new Subject(false, subject.getPrincipals(), subject.getPublicCredentials(), subject.getPrivateCredentials());
-            subject.getPrincipals().add(new TaskPrincipal("Shutdown"));
-            Subject.doAs(subject, new PrivilegedAction<Object>()
+            Subject.doAs(SecurityManager.getSystemTaskSubject("Shutdown"), new PrivilegedAction<Object>()
             {
                 @Override
                 public Object run()
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/VirtualHostMessages.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/VirtualHostMessages.java
index d1a90f0b91..a53927c283 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/VirtualHostMessages.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/VirtualHostMessages.java
@@ -64,16 +64,21 @@ public class VirtualHostMessages
 
     /**
      * Log a VirtualHost message of the Format:
-     * <pre>VHT-1002 : Closed</pre>
+     * <pre>VHT-1002 : Closed : {0}</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *
      */
-    public static LogMessage CLOSED()
+    public static LogMessage CLOSED(String param1)
     {
         String rawMessage = _messages.getString("CLOSED");
 
-        final String message = rawMessage;
+        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()
         {
@@ -187,16 +192,21 @@ public class VirtualHostMessages
 
     /**
      * Log a VirtualHost message of the Format:
-     * <pre>VHT-1005 : Unexpected fatal error</pre>
+     * <pre>VHT-1005 : {0} Unexpected fatal error</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *
      */
-    public static LogMessage ERRORED()
+    public static LogMessage ERRORED(String param1)
     {
         String rawMessage = _messages.getString("ERRORED");
 
-        final String message = rawMessage;
+        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()
         {
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/VirtualHost_logmessages.properties b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/VirtualHost_logmessages.properties
index 5695026cbc..8cd15f0120 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/VirtualHost_logmessages.properties
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/logging/messages/VirtualHost_logmessages.properties
@@ -20,9 +20,9 @@
 #
 # 0 - name
 CREATED = VHT-1001 : Created : {0}
-CLOSED = VHT-1002 : Closed
+CLOSED = VHT-1002 : Closed : {0}
 
 STATS_DATA = VHT-1003 : {0} : {1,choice,0#delivered|1#received} : {2,number,#.###} kB/s peak : {3,number,#} bytes total
 STATS_MSGS = VHT-1004 : {0} : {1,choice,0#delivered|1#received} : {2,number,#.###} msg/s peak : {3,number,#} msgs total
 
-ERRORED = VHT-1005 : Unexpected fatal error
\ No newline at end of file
+ERRORED = VHT-1005 : {0} Unexpected fatal error
\ No newline at end of file
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 6a8405649a..05d907e03d 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
@@ -41,7 +41,6 @@ import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.configuration.updater.TaskExecutor;
 import org.apache.qpid.server.logging.EventLogger;
 import org.apache.qpid.server.logging.LogRecorder;
-import org.apache.qpid.server.logging.MessageLogger;
 import org.apache.qpid.server.logging.messages.BrokerMessages;
 import org.apache.qpid.server.model.*;
 import org.apache.qpid.server.plugin.PreferencesProviderFactory;
@@ -477,7 +476,7 @@ public class BrokerAdapter<X extends Broker<X>> extends AbstractConfiguredObject
 
         // permission has already been granted to create the virtual host
         // disable further access check on other operations, e.g. create exchange
-        Subject.doAs(SecurityManager.SYSTEM, new PrivilegedAction<Object>()
+        Subject.doAs(SecurityManager.getSubjectWithAddedSystemRights(), new PrivilegedAction<Object>()
                             {
                                 @Override
                                 public Object run()
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java
index 663d177541..181568ea5d 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java
@@ -22,6 +22,7 @@ import java.lang.reflect.Type;
 import java.security.AccessControlException;
 import java.security.AccessController;
 import java.security.Principal;
+import java.security.PrivilegedAction;
 import java.util.*;
 import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -55,6 +56,8 @@ import org.apache.qpid.server.message.ServerMessage;
 import org.apache.qpid.server.protocol.AMQSessionModel;
 import org.apache.qpid.server.consumer.Consumer;
 import org.apache.qpid.server.consumer.ConsumerTarget;
+import org.apache.qpid.server.security.*;
+import org.apache.qpid.server.security.SecurityManager;
 import org.apache.qpid.server.security.auth.AuthenticatedPrincipal;
 import org.apache.qpid.server.store.DurableConfigurationStoreHelper;
 import org.apache.qpid.server.store.StorableMessageMetaData;
@@ -985,60 +988,72 @@ public abstract class AbstractQueue
         _totalMessagesReceived.incrementAndGet();
 
 
-        QueueEntry entry;
+
         final QueueConsumer<?> exclusiveSub = _exclusiveSubscriber;
-        entry = _entries.add(message);
+        final QueueEntry entry = _entries.add(message);
 
         if(action != null || (exclusiveSub == null  && _queueRunner.isIdle()))
         {
             /*
-
-            iterate over consumers and if any is at the end of the queue and can deliver this message, then deliver the message
-
+             * iterate over consumers and if any is at the end of the queue and can deliver this message,
+             * then deliver the message
              */
-            QueueConsumerList.ConsumerNode node = _consumerList.getMarkedNode();
-            QueueConsumerList.ConsumerNode nextNode = node.findNext();
-            if (nextNode == null)
-            {
-                nextNode = _consumerList.getHead().findNext();
-            }
-            while (nextNode != null)
-            {
-                if (_consumerList.updateMarkedNode(node, nextNode))
-                {
-                    break;
-                }
-                else
-                {
-                    node = _consumerList.getMarkedNode();
-                    nextNode = node.findNext();
-                    if (nextNode == null)
-                    {
-                        nextNode = _consumerList.getHead().findNext();
-                    }
-                }
-            }
-
-            // always do one extra loop after we believe we've finished
-            // this catches the case where we *just* miss an update
-            int loops = 2;
-
-            while (entry.isAvailable() && loops != 0)
-            {
-                if (nextNode == null)
-                {
-                    loops--;
-                    nextNode = _consumerList.getHead();
-                }
-                else
-                {
-                    // if consumer at end, and active, offer
-                    QueueConsumer<?> sub = nextNode.getConsumer();
-                    deliverToConsumer(sub, entry);
-                }
-                nextNode = nextNode.findNext();
 
-            }
+            Subject.doAs(SecurityManager.getSystemTaskSubject("Immediate Delivery"),
+                         new PrivilegedAction<Object>()
+                         {
+                             @Override
+                             public Object run()
+                             {
+
+                                 QueueConsumerList.ConsumerNode node = _consumerList.getMarkedNode();
+                                 QueueConsumerList.ConsumerNode nextNode = node.findNext();
+                                 if (nextNode == null)
+                                 {
+                                     nextNode = _consumerList.getHead().findNext();
+                                 }
+                                 while (nextNode != null)
+                                 {
+                                     if (_consumerList.updateMarkedNode(node, nextNode))
+                                     {
+                                         break;
+                                     }
+                                     else
+                                     {
+                                         node = _consumerList.getMarkedNode();
+                                         nextNode = node.findNext();
+                                         if (nextNode == null)
+                                         {
+                                             nextNode = _consumerList.getHead().findNext();
+                                         }
+                                     }
+                                 }
+                                 // always do one extra loop after we believe we've finished
+                                 // this catches the case where we *just* miss an update
+                                 int loops = 2;
+
+                                 while (entry.isAvailable() && loops != 0)
+                                 {
+                                     if (nextNode == null)
+                                     {
+                                         loops--;
+                                         nextNode = _consumerList.getHead();
+                                     }
+                                     else
+                                     {
+                                         // if consumer at end, and active, offer
+                                         final QueueConsumer<?> sub = nextNode.getConsumer();
+                                         deliverToConsumer(sub, entry);
+
+
+                                     }
+                                     nextNode = nextNode.findNext();
+
+                                 }
+
+                                 return null;
+                             }
+                         });
         }
 
 
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueConsumerImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueConsumerImpl.java
index ebddc6318d..645262e227 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueConsumerImpl.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueConsumerImpl.java
@@ -92,7 +92,7 @@ class QueueConsumerImpl
     {
         public void stateChanged(QueueConsumerImpl sub, State oldState, State newState)
         {
-            _eventLogger.message(SubscriptionMessages.STATE(newState.toString()));
+            _eventLogger.message(QueueConsumerImpl.this, SubscriptionMessages.STATE(newState.toString()));
         }
     };
     @ManagedAttributeField
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueRunner.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueRunner.java
index 4d21e9d23a..f3f644b2f9 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueRunner.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/QueueRunner.java
@@ -28,6 +28,8 @@ import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 
 import org.apache.log4j.Logger;
+import org.apache.qpid.server.security.*;
+import org.apache.qpid.server.security.SecurityManager;
 import org.apache.qpid.server.security.auth.TaskPrincipal;
 import org.apache.qpid.server.util.ConnectionScopedRuntimeException;
 import org.apache.qpid.transport.TransportException;
@@ -66,10 +68,7 @@ public class QueueRunner implements Runnable
     {
         if(_scheduled.compareAndSet(SCHEDULED,RUNNING))
         {
-            Subject subject = new Subject(false, org.apache.qpid.server.security.SecurityManager.SYSTEM.getPrincipals(), Collections
-                    .emptySet(), Collections.emptySet());
-            subject.getPrincipals().add(new TaskPrincipal("Queue Delivery"));
-            Subject.doAs(subject, new PrivilegedAction<Object>()
+            Subject.doAs(SecurityManager.getSystemTaskSubject("Queue Delivery"), new PrivilegedAction<Object>()
             {
                 @Override
                 public Object run()
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/SubFlushRunner.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/SubFlushRunner.java
index 610c5f8202..73a23cd53e 100755
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/SubFlushRunner.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/queue/SubFlushRunner.java
@@ -62,9 +62,7 @@ class SubFlushRunner implements Runnable
     {
         if(_scheduled.compareAndSet(SCHEDULED, RUNNING))
         {
-            Subject subject = new Subject(false, SecurityManager.SYSTEM.getPrincipals(), Collections.emptySet(), Collections.emptySet());
-            subject.getPrincipals().add(new TaskPrincipal("Sub. Delivery"));
-            Subject.doAs(subject, new PrivilegedAction<Object>()
+            Subject.doAs(SecurityManager.getSystemTaskSubject("Sub. Delivery"), new PrivilegedAction<Object>()
             {
                 @Override
                 public Object run()
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 e3e2ec272c..6add2cef21 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
@@ -151,10 +151,7 @@ public class ApplicationRegistry implements IApplicationRegistry
         {
             _reset = reset;
             _logger = logger;
-            _subject = new Subject(false, SecurityManager.SYSTEM.getPrincipals(), Collections.emptySet(), Collections.emptySet());
-            _subject.getPrincipals().add(new TaskPrincipal("Statistics"));
-            _subject.setReadOnly();
-
+            _subject = SecurityManager.getSystemTaskSubject("Statistics");
         }
 
         public void run()
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java
index ba07fb56a7..c45b7bb2be 100755
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java
@@ -34,6 +34,7 @@ import org.apache.qpid.server.security.access.ObjectProperties;
 import org.apache.qpid.server.security.access.ObjectType;
 import org.apache.qpid.server.security.access.Operation;
 import org.apache.qpid.server.security.access.OperationLoggingDetails;
+import org.apache.qpid.server.security.auth.TaskPrincipal;
 
 import javax.security.auth.Subject;
 
@@ -69,7 +70,7 @@ public class SecurityManager implements ConfigurationChangeListener
 {
     private static final Logger _logger = Logger.getLogger(SecurityManager.class);
 
-    public static final Subject SYSTEM = new Subject(true,
+    private static final Subject SYSTEM = new Subject(true,
                                                      Collections.singleton(new SystemPrincipal()),
                                                      Collections.emptySet(),
                                                      Collections.emptySet());
@@ -107,6 +108,31 @@ public class SecurityManager implements ConfigurationChangeListener
         }
     }
 
+    public static Subject getSubjectWithAddedSystemRights()
+    {
+        Subject subject = Subject.getSubject(AccessController.getContext());
+        if(subject == null)
+        {
+            subject = new Subject();
+        }
+        else
+        {
+            subject = new Subject(false, subject.getPrincipals(), subject.getPublicCredentials(), subject.getPrivateCredentials());
+        }
+        subject.getPrincipals().addAll(SYSTEM.getPrincipals());
+        subject.setReadOnly();
+        return subject;
+    }
+
+
+    public static Subject getSystemTaskSubject(String taskName)
+    {
+        Subject subject = new Subject(false, SYSTEM.getPrincipals(), SYSTEM.getPublicCredentials(), SYSTEM.getPrivateCredentials());
+        subject.getPrincipals().add(new TaskPrincipal(taskName));
+        subject.setReadOnly();
+        return subject;
+    }
+
     private void configureVirtualHostAclPlugin(String aclFile, String vhostName)
     {
         if(aclFile != null)
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
index 650abd360c..a4719f6058 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
@@ -714,7 +714,7 @@ public abstract class AbstractVirtualHost implements VirtualHost, IConnectionReg
 
         _state = State.STOPPED;
 
-        _eventLogger.message(VirtualHostMessages.CLOSED());
+        _eventLogger.message(VirtualHostMessages.CLOSED(getName()));
     }
 
     protected void closeStorage()
@@ -921,7 +921,7 @@ public abstract class AbstractVirtualHost implements VirtualHost, IConnectionReg
     {
         if (state == State.ERRORED)
         {
-            _eventLogger.message(VirtualHostMessages.ERRORED());
+            _eventLogger.message(VirtualHostMessages.ERRORED(getName()));
         }
     }
 
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/HouseKeepingTask.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/HouseKeepingTask.java
index 83ef437410..6e5ac79559 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/HouseKeepingTask.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/HouseKeepingTask.java
@@ -22,6 +22,8 @@ package org.apache.qpid.server.virtualhost;
 
 import org.apache.log4j.Logger;
 
+import org.apache.qpid.server.security.*;
+import org.apache.qpid.server.security.SecurityManager;
 import org.apache.qpid.server.security.auth.TaskPrincipal;
 
 import javax.security.auth.Subject;
@@ -42,11 +44,7 @@ public abstract class HouseKeepingTask implements Runnable
     {
         _virtualHost = vhost;
         _name = _virtualHost.getName() + ":" + this.getClass().getSimpleName();
-        _subject = new Subject(false, org.apache.qpid.server.security.SecurityManager.SYSTEM.getPrincipals(), Collections
-                .emptySet(), Collections.emptySet());
-        _subject.getPrincipals().add(new TaskPrincipal(_name));
-        _subject.setReadOnly();
-
+        _subject = SecurityManager.getSystemTaskSubject(_name);
     }
 
     final public void run()
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/configuration/startup/KeyStoreRecovererTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/configuration/startup/KeyStoreRecovererTest.java
index 1a204292b7..55dc225326 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/configuration/startup/KeyStoreRecovererTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/configuration/startup/KeyStoreRecovererTest.java
@@ -63,7 +63,7 @@ public class KeyStoreRecovererTest extends TestCase
         assertEquals(id, keyStore.getId());
 
         //verify we can retrieve the actual password using the method
-        Subject.doAs(SecurityManager.SYSTEM, new PrivilegedAction<Object>()
+        Subject.doAs(SecurityManager.getSubjectWithAddedSystemRights(), new PrivilegedAction<Object>()
         {
             @Override
             public Object run()
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/configuration/startup/TrustStoreRecovererTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/configuration/startup/TrustStoreRecovererTest.java
index 6ba53007dd..5e0ce9dbc4 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/configuration/startup/TrustStoreRecovererTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/configuration/startup/TrustStoreRecovererTest.java
@@ -59,7 +59,7 @@ public class TrustStoreRecovererTest extends QpidTestCase
         assertNotNull("Trust store configured object is not created", trustStore);
         assertEquals(id, trustStore.getId());
 
-        Subject.doAs(SecurityManager.SYSTEM, new PrivilegedAction<Object>()
+        Subject.doAs(SecurityManager.getSubjectWithAddedSystemRights(), new PrivilegedAction<Object>()
         {
             @Override
             public Object run()
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/logging/messages/VirtualHostMessagesTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/logging/messages/VirtualHostMessagesTest.java
index c1068b4a0b..5a272ff481 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/logging/messages/VirtualHostMessagesTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/logging/messages/VirtualHostMessagesTest.java
@@ -40,10 +40,11 @@ public class VirtualHostMessagesTest extends AbstractTestMessages
 
     public void testVirtualhostClosed()
     {
-        _logMessage = VirtualHostMessages.CLOSED();
+        String name = "test";
+        _logMessage = VirtualHostMessages.CLOSED(name);
         List<Object> log = performLog();
 
-        String[] expected = {"Closed"};
+        String[] expected = {"Closed :", name};
 
         validateLogMessage(log, "VHT-1002", expected);
     }
diff --git a/qpid/java/broker-core/src/velocity/templates/org/apache/qpid/server/logging/messages/LogMessages.vm b/qpid/java/broker-core/src/velocity/templates/org/apache/qpid/server/logging/messages/LogMessages.vm
index 803c2151da..6a97f7facd 100644
--- a/qpid/java/broker-core/src/velocity/templates/org/apache/qpid/server/logging/messages/LogMessages.vm
+++ b/qpid/java/broker-core/src/velocity/templates/org/apache/qpid/server/logging/messages/LogMessages.vm
@@ -20,7 +20,7 @@
  */
 package ${package};
 
-import static org.apache.qpid.server.logging.AbstractRootMessageLogger.DEFAULT_LOG_HIERARCHY_PREFIX;
+import static org.apache.qpid.server.logging.AbstractMessageLogger.DEFAULT_LOG_HIERARCHY_PREFIX;
 
 import org.apache.log4j.Logger;
 import org.apache.qpid.server.configuration.BrokerProperties;
diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
index b20685985f..7b0a48cac1 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
@@ -222,11 +222,7 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler
         {
             try
             {
-                Subject subject = Subject.getSubject(AccessController.getContext());
-                subject = new Subject(false, subject.getPrincipals(), subject.getPublicCredentials(), subject.getPrivateCredentials());
-                subject.getPrincipals().addAll(SecurityManager.SYSTEM.getPrincipals());
-
-                return Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
+                return Subject.doAs(SecurityManager.getSubjectWithAddedSystemRights(), new PrivilegedExceptionAction<Object>()
                 {
                     @Override
                     public Object run() throws IllegalAccessException, InvocationTargetException
diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporter.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporter.java
index f99fe89f7b..c1792f0227 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporter.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporter.java
@@ -38,7 +38,6 @@ import org.apache.qpid.server.security.auth.jmx.JMXConnectionPrincipal;
 import java.rmi.server.RemoteServer;
 import java.rmi.server.ServerNotActiveException;
 import java.security.PrivilegedAction;
-import java.util.Collections;
 
 public class ManagementLogonLogoffReporter implements  NotificationListener, NotificationFilter
 {
@@ -63,31 +62,45 @@ public class ManagementLogonLogoffReporter implements  NotificationListener, Not
             LOGGER.debug("Notification connectionId : " + connectionId + " type : " + type);
         }
 
-        String user = _usernameAccessor.getUsernameForConnectionId(connectionId);
+        Subject subject = _usernameAccessor.getSubjectConnectionId(connectionId);
+        if(subject == null)
+        {
+            subject = new Subject();
+        }
+        AuthenticatedPrincipal authenticatedPrincipal =
+                AuthenticatedPrincipal.getOptionalAuthenticatedPrincipalFromSubject(subject);
 
-        // If user is still null, fallback to an unordered list of Principals from the connection id.
-        if (user == null)
+        String user;
+
+        if(authenticatedPrincipal != null)
         {
+            user = authenticatedPrincipal.getName();
+        }
+        else
+        {
+            // If user is still null, fallback to an unordered list of Principals from the connection id.
             final String[] splitConnectionId = connectionId.split(" ");
             user = splitConnectionId[1];
         }
-        Subject originalSubject = new Subject(false, Collections.singleton(new AuthenticatedPrincipal(user)), Collections.emptySet(), Collections.emptySet());
-        Subject subject;
 
-        try
-        {
-            String clientHost = RemoteServer.getClientHost();
-            subject = new Subject(false,
-                                  originalSubject.getPrincipals(),
-                                  originalSubject.getPublicCredentials(),
-                                  originalSubject.getPrivateCredentials());
-            subject.getPrincipals().add(new JMXConnectionPrincipal(clientHost));
-            subject.setReadOnly();
-        }
-        catch(ServerNotActiveException e)
+
+        if(subject.getPrincipals(JMXConnectionPrincipal.class).isEmpty())
         {
-            subject = originalSubject;
+            try
+            {
+                String clientHost = RemoteServer.getClientHost();
+                subject = new Subject(false,
+                                      subject.getPrincipals(),
+                                      subject.getPublicCredentials(),
+                                      subject.getPrivateCredentials());
+                subject.getPrincipals().add(new JMXConnectionPrincipal(clientHost));
+                subject.setReadOnly();
+            }
+            catch(ServerNotActiveException e)
+            {
+            }
         }
+
         final String username = user;
         Subject.doAs(subject, new PrivilegedAction<Object>()
         {
diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameAccessor.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameAccessor.java
index 0cbb0d2687..18ab02ece1 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameAccessor.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameAccessor.java
@@ -19,8 +19,10 @@
  */
 package org.apache.qpid.server.jmx;
 
+import javax.security.auth.Subject;
+
 public interface UsernameAccessor
 {
-    public String getUsernameForConnectionId(String connectionId);
+    public Subject getSubjectConnectionId(String connectionId);
 
 }
diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameCachingRMIJRMPServer.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameCachingRMIJRMPServer.java
index 838e9e5664..4caa14014a 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameCachingRMIJRMPServer.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/UsernameCachingRMIJRMPServer.java
@@ -37,11 +37,9 @@ import javax.management.remote.rmi.RMIConnection;
 import javax.management.remote.rmi.RMIJRMPServerImpl;
 import javax.security.auth.Subject;
 
-import org.apache.qpid.server.security.auth.AuthenticatedPrincipal;
-
 /**
  * An implementation of RMIJRMPServerImpl that caches the usernames of users as they log-on
- * and makes the same available via {@link UsernameAccessor#getUsernameForConnectionId(String)}.
+ * and makes the same available via {@link UsernameAccessor#getSubjectConnectionId(String)}.
  *
  * Caller is responsible for installing this object as a {@link NotificationListener} of the
  * {@link JMXConnectorServer} so the cache entries are removed as the clients disconnect.
@@ -50,7 +48,7 @@ import org.apache.qpid.server.security.auth.AuthenticatedPrincipal;
 public class UsernameCachingRMIJRMPServer extends RMIJRMPServerImpl implements NotificationListener, NotificationFilter, UsernameAccessor
 {
     // ConnectionId is guaranteed to be unique per client connection, according to the JMX spec.
-    private final Map<String, String> _connectionIdUsernameMap = new ConcurrentHashMap<String, String>();
+    private final Map<String, Subject> _connectionIdUsernameMap = new ConcurrentHashMap<String, Subject>();
 
     UsernameCachingRMIJRMPServer(int port, RMIClientSocketFactory csf, RMIServerSocketFactory ssf,
             Map<String, ?> env) throws IOException
@@ -62,13 +60,12 @@ public class UsernameCachingRMIJRMPServer extends RMIJRMPServerImpl implements N
     protected RMIConnection makeClient(String connectionId, Subject subject) throws IOException
     {
         final RMIConnection makeClient = super.makeClient(connectionId, subject);
-        final AuthenticatedPrincipal authenticatedPrincipalFromSubject = AuthenticatedPrincipal.getAuthenticatedPrincipalFromSubject(subject);
-        _connectionIdUsernameMap.put(connectionId, authenticatedPrincipalFromSubject.getName());
+        _connectionIdUsernameMap.put(connectionId, subject);
         return makeClient;
     }
 
     @Override
-    public String getUsernameForConnectionId(String connectionId)
+    public Subject getSubjectConnectionId(String connectionId)
     {
         return _connectionIdUsernameMap.get(connectionId);
     }
diff --git a/qpid/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporterTest.java b/qpid/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporterTest.java
index 0027815142..be5fe1eeca 100644
--- a/qpid/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporterTest.java
+++ b/qpid/java/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/ManagementLogonLogoffReporterTest.java
@@ -31,18 +31,22 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 
 import javax.management.remote.JMXConnectionNotification;
+import javax.security.auth.Subject;
 
 import org.apache.qpid.server.logging.EventLogger;
 import org.apache.qpid.server.logging.LogMessage;
 import org.apache.qpid.server.logging.MessageLogger;
 
 import junit.framework.TestCase;
+import org.apache.qpid.server.security.auth.AuthenticatedPrincipal;
 import org.mockito.ArgumentMatcher;
 
+import java.util.Collections;
+
 public class ManagementLogonLogoffReporterTest extends TestCase
 {
     private static final String TEST_JMX_UNIQUE_CONNECTION_ID = "jmxconnectionid1 jmxuser,group";
-    private static final String TEST_USER = "jmxuser";
+    private static final Subject TEST_USER = new Subject(false, Collections.singleton(new AuthenticatedPrincipal("jmxuser")), Collections.emptySet(), Collections.emptySet());
 
     private ManagementLogonLogoffReporter _reporter;
     private UsernameAccessor _usernameAccessor;
@@ -62,7 +66,7 @@ public class ManagementLogonLogoffReporterTest extends TestCase
 
     public void testOpenedNotification()
     {
-        when(_usernameAccessor.getUsernameForConnectionId(TEST_JMX_UNIQUE_CONNECTION_ID)).thenReturn(TEST_USER);
+        when(_usernameAccessor.getSubjectConnectionId(TEST_JMX_UNIQUE_CONNECTION_ID)).thenReturn(TEST_USER);
         JMXConnectionNotification openNotification = createMockNotification(TEST_JMX_UNIQUE_CONNECTION_ID, OPENED);
 
         _reporter.handleNotification(openNotification, null);
@@ -86,7 +90,7 @@ public class ManagementLogonLogoffReporterTest extends TestCase
 
     public void testClosedNotification()
     {
-        when(_usernameAccessor.getUsernameForConnectionId(TEST_JMX_UNIQUE_CONNECTION_ID)).thenReturn(TEST_USER);
+        when(_usernameAccessor.getSubjectConnectionId(TEST_JMX_UNIQUE_CONNECTION_ID)).thenReturn(TEST_USER);
         JMXConnectionNotification closeNotification = createMockNotification(TEST_JMX_UNIQUE_CONNECTION_ID, CLOSED);
 
         _reporter.handleNotification(closeNotification, null);
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/InternalBrokerHolder.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/InternalBrokerHolder.java
index c2713a5e1f..f980453d49 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/InternalBrokerHolder.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/InternalBrokerHolder.java
@@ -27,6 +27,7 @@ import org.apache.log4j.Logger;
 
 import org.apache.qpid.server.Broker;
 import org.apache.qpid.server.security.auth.TaskPrincipal;
+import org.apache.qpid.server.security.SecurityManager;
 
 import javax.security.auth.Subject;
 
@@ -61,11 +62,7 @@ public class InternalBrokerHolder implements BrokerHolder
     {
         LOGGER.info("Shutting down Broker instance");
 
-        Subject subject = org.apache.qpid.server.security.SecurityManager.SYSTEM;
-        subject = new Subject(false, subject.getPrincipals(), subject.getPublicCredentials(), subject.getPrivateCredentials());
-        subject.getPrincipals().add(new TaskPrincipal("Shutdown"));
-
-        Subject.doAs(subject, new PrivilegedAction<Object>()
+        Subject.doAs(SecurityManager.getSystemTaskSubject("Shutdown"), new PrivilegedAction<Object>()
         {
             @Override
             public Object run()