Fix non-deterministic NPE in BaseLogger due to incorrect static field initialization

6 files changed, 103 insertions, 39 deletions

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/BaseLogger.java b/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/BaseLogger.java
index f4c7e3c95f9..fec49378945 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/BaseLogger.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/BaseLogger.java
@@ -24,127 +24,127 @@ import javax.annotation.Nullable;
 abstract class BaseLogger implements Logger {
   @Override
   public void trace(String msg) {
-    LogInterceptor.instance().log(msg);
+    LogInterceptors.get().log(msg);
     doTrace(msg);
   }
 
   @Override
   public void trace(String pattern, @Nullable Object arg) {
-    LogInterceptor.instance().log(pattern, arg);
+    LogInterceptors.get().log(pattern, arg);
     doTrace(pattern, arg);
   }
 
   @Override
   public void trace(String msg, @Nullable Object arg1, @Nullable Object arg2) {
-    LogInterceptor.instance().log(msg, arg1, arg2);
+    LogInterceptors.get().log(msg, arg1, arg2);
     doTrace(msg, arg1, arg2);
   }
 
   @Override
   public void trace(String msg, Object... args) {
-    LogInterceptor.instance().log(msg, args);
+    LogInterceptors.get().log(msg, args);
     doTrace(msg, args);
   }
 
   @Override
   public void debug(String msg) {
-    LogInterceptor.instance().log(msg);
+    LogInterceptors.get().log(msg);
     doDebug(msg);
   }
 
   @Override
   public void debug(String pattern, @Nullable Object arg) {
-    LogInterceptor.instance().log(pattern, arg);
+    LogInterceptors.get().log(pattern, arg);
     doDebug(pattern, arg);
   }
 
   @Override
   public void debug(String msg, @Nullable Object arg1, @Nullable Object arg2) {
-    LogInterceptor.instance().log(msg, arg1, arg2);
+    LogInterceptors.get().log(msg, arg1, arg2);
     doDebug(msg, arg1, arg2);
   }
 
   @Override
   public void debug(String msg, Object... args) {
-    LogInterceptor.instance().log(msg, args);
+    LogInterceptors.get().log(msg, args);
     doDebug(msg, args);
   }
 
   @Override
   public void info(String msg) {
-    LogInterceptor.instance().log(msg);
+    LogInterceptors.get().log(msg);
     doInfo(msg);
   }
 
   @Override
   public void info(String msg, @Nullable Object arg) {
-    LogInterceptor.instance().log(msg, arg);
+    LogInterceptors.get().log(msg, arg);
     doInfo(msg, arg);
   }
 
   @Override
   public void info(String msg, @Nullable Object arg1, @Nullable Object arg2) {
-    LogInterceptor.instance().log(msg, arg1, arg2);
+    LogInterceptors.get().log(msg, arg1, arg2);
     doInfo(msg, arg1, arg2);
   }
 
   @Override
   public void info(String msg, Object... args) {
-    LogInterceptor.instance().log(msg, args);
+    LogInterceptors.get().log(msg, args);
     doInfo(msg, args);
   }
 
   @Override
   public void warn(String msg) {
-    LogInterceptor.instance().log(msg);
+    LogInterceptors.get().log(msg);
     doWarn(msg);
   }
 
   @Override
   public void warn(String msg, @Nullable Object arg) {
-    LogInterceptor.instance().log(msg, arg);
+    LogInterceptors.get().log(msg, arg);
     doWarn(msg, arg);
   }
 
   @Override
   public void warn(String msg, @Nullable Object arg1, @Nullable Object arg2) {
-    LogInterceptor.instance().log(msg, arg1, arg2);
+    LogInterceptors.get().log(msg, arg1, arg2);
     doWarn(msg, arg1, arg2);
   }
 
   @Override
   public void warn(String msg, Object... args) {
-    LogInterceptor.instance().log(msg, args);
+    LogInterceptors.get().log(msg, args);
     doWarn(msg, args);
   }
 
   @Override
   public void error(String msg) {
-    LogInterceptor.instance().log(msg);
+    LogInterceptors.get().log(msg);
     doError(msg);
   }
 
   @Override
   public void error(String msg, @Nullable Object arg) {
-    LogInterceptor.instance().log(msg, arg);
+    LogInterceptors.get().log(msg, arg);
     doError(msg, arg);
   }
 
   @Override
   public void error(String msg, @Nullable Object arg1, @Nullable Object arg2) {
-    LogInterceptor.instance().log(msg, arg1, arg2);
+    LogInterceptors.get().log(msg, arg1, arg2);
     doError(msg, arg1, arg2);
   }
 
   @Override
   public void error(String msg, Object... args) {
-    LogInterceptor.instance().log(msg, args);
+    LogInterceptors.get().log(msg, args);
     doError(msg, args);
   }
 
   @Override
   public void error(String msg, Throwable thrown) {
-    LogInterceptor.instance().log(msg, thrown);
+    LogInterceptors.get().log(msg, thrown);
     doError(msg, thrown);
   }
 
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/LogInterceptor.java b/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/LogInterceptor.java
index ff844379d9e..49701a01df3 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/LogInterceptor.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/LogInterceptor.java
@@ -19,12 +19,8 @@
  */
 package org.sonar.api.utils.log;
 
-import com.google.common.base.Preconditions;
-
 abstract class LogInterceptor {
 
-  private static LogInterceptor instance = NullInterceptor.NULL_INSTANCE;
-
   abstract void log(String msg);
 
   abstract void log(String msg, Object arg);
@@ -34,13 +30,4 @@ abstract class LogInterceptor {
   abstract void log(String msg, Object... args);
 
   abstract void log(String msg, Throwable thrown);
-
-  static LogInterceptor instance() {
-    return instance;
-  }
-
-  static void setInstance(LogInterceptor li) {
-    Preconditions.checkArgument(li != null);
-    instance = li;
-  }
 }
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/LogInterceptors.java b/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/LogInterceptors.java
new file mode 100644
index 00000000000..a17ab972167
--- /dev/null
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/LogInterceptors.java
@@ -0,0 +1,39 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2014 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * SonarQube is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * SonarQube is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.api.utils.log;
+
+import com.google.common.base.Preconditions;
+
+class LogInterceptors {
+
+  private static LogInterceptor instance = NullInterceptor.NULL_INSTANCE;
+
+  private LogInterceptors() {
+  }
+
+  static LogInterceptor get() {
+    return instance;
+  }
+
+  static void set(LogInterceptor li) {
+    Preconditions.checkArgument(li != null);
+    instance = li;
+  }
+}
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/LogTester.java b/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/LogTester.java
index ac63133bfc2..630492277ce 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/LogTester.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/utils/log/LogTester.java
@@ -65,13 +65,13 @@ public class LogTester extends ExternalResource {
     initialLevel = Loggers.getFactory().getLevel();
 
     // this shared instance breaks compatibility with parallel execution of tests
-    LogInterceptor.setInstance(new ListInterceptor());
+    LogInterceptors.set(new ListInterceptor());
     setLevel(LoggerLevel.TRACE);
   }
 
   @Override
   protected void after() {
-    LogInterceptor.setInstance(NullInterceptor.NULL_INSTANCE);
+    LogInterceptors.set(NullInterceptor.NULL_INSTANCE);
     setLevel(initialLevel);
   }
 
@@ -92,11 +92,11 @@ public class LogTester extends ExternalResource {
    * Logs in chronological order (item at index 0 is the oldest one)
    */
   public List<String> logs() {
-    return ((ListInterceptor) LogInterceptor.instance()).logs();
+    return ((ListInterceptor) LogInterceptors.get()).logs();
   }
 
   public LogTester clear() {
-    ((ListInterceptor) LogInterceptor.instance()).clear();
+    ((ListInterceptor) LogInterceptors.get()).clear();
     return this;
   }
 }
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/utils/log/LogInterceptorsTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/utils/log/LogInterceptorsTest.java
new file mode 100644
index 00000000000..7a5adbe5ca9
--- /dev/null
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/utils/log/LogInterceptorsTest.java
@@ -0,0 +1,38 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2014 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * SonarQube is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * SonarQube is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.api.utils.log;
+
+import org.junit.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class LogInterceptorsTest {
+
+  @Test
+  public void default_is_null_interceptor() throws Exception {
+    // production-ready
+    assertThat(LogInterceptors.get()).isInstanceOf(NullInterceptor.class);
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void instance_cant_be_null() throws Exception {
+    LogInterceptors.set(null);
+  }
+}
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/utils/log/LogTesterTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/utils/log/LogTesterTest.java
index 8b10230815b..9fd01343f34 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/utils/log/LogTesterTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/utils/log/LogTesterTest.java
@@ -59,6 +59,6 @@ public class LogTesterTest {
     assertThat(sut.logs()).isEmpty();
 
     sut.after();
-    assertThat(LogInterceptor.instance()).isSameAs(NullInterceptor.NULL_INSTANCE);
+    assertThat(LogInterceptors.get()).isSameAs(NullInterceptor.NULL_INSTANCE);
   }
 }