Unset logging context before returning a thread back to the thread pool

After doing work in a thread and before returning it to the thread pool
we must clean up its LoggingContext. To do this we currently remember
the existing LoggingContext when we get the thread and set it back when
we return the thread.

Since we expect that the provided thread always has an empty
LoggingContext (and it's a bug if it's non-empty), it's simpler and
safer to just unset the LoggingContext before we return the thread to
the thread pool.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I4cf614068679831c6dfdcc794c7e6beb0e66be47
diff --git a/java/com/google/gerrit/server/logging/LoggingContextAwareCallable.java b/java/com/google/gerrit/server/logging/LoggingContextAwareCallable.java
index 3906630..63a1f88 100644
--- a/java/com/google/gerrit/server/logging/LoggingContextAwareCallable.java
+++ b/java/com/google/gerrit/server/logging/LoggingContextAwareCallable.java
@@ -14,7 +14,6 @@
 
 package com.google.gerrit.server.logging;
 
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSetMultimap;
 import java.util.concurrent.Callable;
 
@@ -65,11 +64,6 @@
 
     // propagate logging context
     LoggingContext loggingCtx = LoggingContext.getInstance();
-    ImmutableSetMultimap<String, String> oldTags = loggingCtx.getTagsAsMap();
-    boolean oldForceLogging = loggingCtx.isLoggingForced();
-    boolean oldPerformanceLogging = loggingCtx.isPerformanceLogging();
-    ImmutableList<PerformanceLogRecord> oldPerformanceLogRecords =
-        loggingCtx.getPerformanceLogRecords();
     loggingCtx.setTags(tags);
     loggingCtx.forceLogging(forceLogging);
     loggingCtx.performanceLogging(performanceLogging);
@@ -84,10 +78,11 @@
     try {
       return callable.call();
     } finally {
-      loggingCtx.setTags(oldTags);
-      loggingCtx.forceLogging(oldForceLogging);
-      loggingCtx.performanceLogging(oldPerformanceLogging);
-      loggingCtx.setPerformanceLogRecords(oldPerformanceLogRecords);
+      // Cleanup logging context. This is important if the thread is pooled and reused.
+      loggingCtx.clearTags();
+      loggingCtx.forceLogging(false);
+      loggingCtx.performanceLogging(false);
+      loggingCtx.clearPerformanceLogEntries();
     }
   }
 }
diff --git a/java/com/google/gerrit/server/logging/LoggingContextAwareRunnable.java b/java/com/google/gerrit/server/logging/LoggingContextAwareRunnable.java
index 712459b..9d9e8c5 100644
--- a/java/com/google/gerrit/server/logging/LoggingContextAwareRunnable.java
+++ b/java/com/google/gerrit/server/logging/LoggingContextAwareRunnable.java
@@ -14,7 +14,6 @@
 
 package com.google.gerrit.server.logging;
 
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSetMultimap;
 
 /**
@@ -88,11 +87,6 @@
 
     // propagate logging context
     LoggingContext loggingCtx = LoggingContext.getInstance();
-    ImmutableSetMultimap<String, String> oldTags = loggingCtx.getTagsAsMap();
-    boolean oldForceLogging = loggingCtx.isLoggingForced();
-    boolean oldPerformanceLogging = loggingCtx.isPerformanceLogging();
-    ImmutableList<PerformanceLogRecord> oldPerformanceLogRecords =
-        loggingCtx.getPerformanceLogRecords();
     loggingCtx.setTags(tags);
     loggingCtx.forceLogging(forceLogging);
     loggingCtx.performanceLogging(performanceLogging);
@@ -107,10 +101,11 @@
     try {
       runnable.run();
     } finally {
-      loggingCtx.setTags(oldTags);
-      loggingCtx.forceLogging(oldForceLogging);
-      loggingCtx.performanceLogging(oldPerformanceLogging);
-      loggingCtx.setPerformanceLogRecords(oldPerformanceLogRecords);
+      // Cleanup logging context. This is important if the thread is pooled and reused.
+      loggingCtx.clearTags();
+      loggingCtx.forceLogging(false);
+      loggingCtx.performanceLogging(false);
+      loggingCtx.clearPerformanceLogEntries();
     }
   }
 }