1 files changed, 26 insertions, 24 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/actors/ManagementActor.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/actors/ManagementActor.java
index 9cd3c66629..a2f3506502 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/actors/ManagementActor.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/actors/ManagementActor.java
@@ -30,16 +30,7 @@ import java.text.MessageFormat;
 import java.util.Set;
 
 /**
- * NOTE: This actor is not thread safe.
- *
- * Sharing of a ManagementActor instance between threads may result in an
- * incorrect actor value being logged.
- *
- * This is due to the fact that calls to message will dynamically query the
- * thread name and use that to set the log format during each message() call.
- *
- * This is currently not an issue as each MBean operation creates a new Actor
- * that is unique for each operation.
+ * Management actor to use in {@link MBeanInvocationHandlerImpl} to log all management operational logging.
  */
 public class ManagementActor extends AbstractActor
 {
@@ -66,38 +57,45 @@ public class ManagementActor extends AbstractActor
     /**
      * The logString to be used for logging
      */
-    private String _logString;
+    private String _logStringContainingPrincipal;
+
+    /** used when the principal name cannot be discovered from the Subject */
+    private final String _fallbackPrincipalName;
 
     /** @param rootLogger The RootLogger to use for this Actor */
     public ManagementActor(RootMessageLogger rootLogger)
     {
         super(rootLogger);
+        _fallbackPrincipalName = UNKNOWN_PRINCIPAL;
+    }
+
+    public ManagementActor(RootMessageLogger rootLogger, String principalName)
+    {
+        super(rootLogger);
+        _fallbackPrincipalName = principalName;
     }
 
-    private void updateLogString()
+    private synchronized String getAndCacheLogString()
     {
         String currentName = Thread.currentThread().getName();
 
         String actor;
+        String logString = _logStringContainingPrincipal;
+
         // Record the last thread name so we don't have to recreate the log string
-        if (!currentName.equals(_lastThreadName))
+        if (_logStringContainingPrincipal == null || !currentName.equals(_lastThreadName))
         {
             _lastThreadName = currentName;
+            String principalName = getPrincipalName();
 
             // Management Thread names have this format.
             // RMI TCP Connection(2)-169.24.29.116
             // This is true for both LocalAPI and JMX Connections
             // However to be defensive lets test.
-
             String[] split = currentName.split("\\(");
             if (split.length == 2)
             {
                 String ip = currentName.split("-")[1];
-                String principalName = getPrincipalName();
-                if (principalName == null)
-                {
-                    principalName = UNKNOWN_PRINCIPAL;
-                }
                 actor = MessageFormat.format(MANAGEMENT_FORMAT, principalName, ip);
             }
             else
@@ -111,9 +109,14 @@ public class ManagementActor extends AbstractActor
                 actor = currentName;
             }
 
-            _logString = "[" + actor + "] ";
+            logString = "[" + actor + "] ";
+            if(principalName != UNKNOWN_PRINCIPAL )
+            {
+                _logStringContainingPrincipal = logString;
+            }
 
         }
+        return logString;
     }
 
     /**
@@ -121,9 +124,9 @@ public class ManagementActor extends AbstractActor
      *
      * @return principal name or null if principal can not be found
      */
-    protected String getPrincipalName()
+    private String getPrincipalName()
     {
-        String identity = null;
+        String identity = _fallbackPrincipalName;
 
         // retrieve Subject from current AccessControlContext
         final Subject subject = Subject.getSubject(AccessController.getContext());
@@ -142,8 +145,7 @@ public class ManagementActor extends AbstractActor
 
     public String getLogMessage()
     {
-        updateLogString();
-        return _logString;
+        return getAndCacheLogString();
     }
 
 }