QPID-4172: HouseKeepingTask now reverts thread name before exiting to reduce confusion when inspecting thread dumps.

Applied patch from Philip Harvey <phil@philharveyonline.com> and Oleksandr Rudyy<orudyy@gmail.com>

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

2 files changed, 51 insertions, 2 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/HouseKeepingTask.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/HouseKeepingTask.java
index 523bafb8e1..1b0e50fd34 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/HouseKeepingTask.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/HouseKeepingTask.java
@@ -44,9 +44,9 @@ public abstract class HouseKeepingTask implements Runnable
 
     final public void run()
     {
-        // Don't need to undo this as this is a thread pool thread so will
-        // always go through here before we do any real work.
+        String originalThreadName = Thread.currentThread().getName();
         Thread.currentThread().setName(_name);
+
         CurrentActor.set(new AbstractActor(_rootLogger)
         {
             @Override
@@ -67,6 +67,9 @@ public abstract class HouseKeepingTask implements Runnable
         finally
         {
             CurrentActor.remove();
+
+            // eagerly revert the thread name to make thread dumps more meaningful if captured after task has finished
+            Thread.currentThread().setName(originalThreadName);
         }
     }
 
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/HouseKeepingTaskTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/HouseKeepingTaskTest.java
index 0794154e47..8b4a52bb79 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/HouseKeepingTaskTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/HouseKeepingTaskTest.java
@@ -64,4 +64,50 @@ public class HouseKeepingTaskTest extends QpidTestCase
         //clean up the test actor
         CurrentActor.remove();
     }
+
+    public void testThreadNameIsSetForDurationOfTask() throws Exception
+    {
+        //create and set a test actor
+        LogActor testActor = new TestLogActor(new NullRootMessageLogger());
+        CurrentActor.set(testActor);
+
+        String originalThreadName = Thread.currentThread().getName();
+
+        String vhostName = "HouseKeepingTaskTestVhost";
+
+        String expectedThreadNameDuringExecution = vhostName + ":" + "ThreadNameRememberingTask";
+
+        ThreadNameRememberingTask testTask = new ThreadNameRememberingTask(new MockVirtualHost(vhostName));
+
+        testTask.run();
+
+        assertEquals("Thread name should have been set during execution", expectedThreadNameDuringExecution, testTask.getThreadNameDuringExecution());
+        assertEquals("Thread name should have been reverted after task has run", originalThreadName, Thread.currentThread().getName());
+
+        //clean up the test actor
+        CurrentActor.remove();
+    }
+
+
+    private static final class ThreadNameRememberingTask extends HouseKeepingTask
+    {
+        private String _threadNameDuringExecution;
+
+        private ThreadNameRememberingTask(VirtualHost vhost)
+        {
+            super(vhost);
+        }
+
+        @Override
+        public void execute()
+        {
+            _threadNameDuringExecution = Thread.currentThread().getName(); // store current thread name so we can assert it later
+            throw new RuntimeException("deliberate exception to check that thread name still gets reverted");
+        }
+
+        public String getThreadNameDuringExecution()
+        {
+            return _threadNameDuringExecution;
+        }
+    }
 }