Avoid calling User.current() from `JobConfigHistorySaveableListener…

…` during Jenkins initialization When `jobConfigHistory` is used used together with the `sonar` (2.2) plugin, we get the following stack: ``` hudson.ExtensionFinder$GuiceFinder$FaultTolerantScope$1 error WARNING: Failed to instantiate Key[type=hudson.plugins.sonar.SonarPublisher$DescriptorImpl, annotation=[none]]; skipping this component com.google.inject.ProvisionException: Guice provision errors: 1) Tried proxying hudson.plugins.sonar.SonarPublisher$DescriptorImpl to support a circular dependency, but it is not an interface. 1 error at com.google.inject.internal.ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:52) at com.google.inject.Scopes$1$1.get(Scopes.java:65) at hudson.ExtensionFinder$GuiceFinder$FaultTolerantScope$1.get(ExtensionFinder.java:427) at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:41) at com.google.inject.internal.InjectorImpl$3$1.call(InjectorImpl.java:1005) at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1058) at com.google.inject.internal.InjectorImpl$3.get(InjectorImpl.java:1001) at hudson.ExtensionFinder$GuiceFinder._find(ExtensionFinder.java:389) at hudson.ExtensionFinder$GuiceFinder.find(ExtensionFinder.java:380) at hudson.ClassicPluginStrategy.findComponents(ClassicPluginStrategy.java:370) at hudson.ExtensionList.load(ExtensionList.java:300) at hudson.ExtensionList.ensureLoaded(ExtensionList.java:253) at hudson.ExtensionList.getComponents(ExtensionList.java:154) at hudson.DescriptorExtensionList.load(DescriptorExtensionList.java:182) at hudson.ExtensionList.ensureLoaded(ExtensionList.java:253) at hudson.ExtensionList.iterator(ExtensionList.java:143) at hudson.model.User.load(User.java:192) at hudson.model.User.<init>(User.java:143) at hudson.model.User.getOrCreate(User.java:443) at hudson.model.User.current(User.java:487) at hudson.plugins.jobConfigHistory.PluginUtils.getHistoryDao(PluginUtils.java:71) at hudson.plugins.jobConfigHistory.PluginUtils.getHistoryDao(PluginUtils.java:62) at hudson.plugins.jobConfigHistory.JobConfigHistorySaveableListener.getHistoryDao(JobConfigHistorySaveableListener.java:49) at hudson.plugins.jobConfigHistory.JobConfigHistorySaveableListener.onChange(JobConfigHistorySaveableListener.java:28) at hudson.model.listeners.SaveableListener.fireOnChange(SaveableListener.java:80) at hudson.model.Descriptor.save(Descriptor.java:760) at hudson.plugins.sonar.SonarPublisher$DescriptorImpl.<init>(SonarPublisher.java:420) at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at java.lang.reflect.Constructor.newInstance(Constructor.java:526) at com.google.inject.internal.DefaultConstructionProxyFactory$1.newInstance(DefaultConstructionProxyFactory.java:86) at com.google.inject.internal.ConstructorInjector.provision(ConstructorInjector.java:108) at com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:88) at com.google.inject.internal.ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:269) at com.google.inject.internal.ProviderToInternalFactoryAdapter$1.call(ProviderToInternalFactoryAdapter.java:46) at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1058) at com.google.inject.internal.ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:40) at com.google.inject.Scopes$1$1.get(Scopes.java:65) at hudson.ExtensionFinder$GuiceFinder$FaultTolerantScope$1.get(ExtensionFinder.java:427) at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:41) at com.google.inject.internal.InjectorImpl$3$1.call(InjectorImpl.java:1005) at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1051) at com.google.inject.internal.InjectorImpl$3.get(InjectorImpl.java:1001) at hudson.ExtensionFinder$GuiceFinder._find(ExtensionFinder.java:389) at hudson.ExtensionFinder$GuiceFinder.find(ExtensionFinder.java:380) at hudson.ClassicPluginStrategy.findComponents(ClassicPluginStrategy.java:370) at hudson.ExtensionList.load(ExtensionList.java:300) at hudson.ExtensionList.ensureLoaded(ExtensionList.java:253) at hudson.ExtensionList.getComponents(ExtensionList.java:154) at hudson.DescriptorExtensionList.load(DescriptorExtensionList.java:182) at hudson.ExtensionList.ensureLoaded(ExtensionList.java:253) at hudson.ExtensionList.iterator(ExtensionList.java:143) at org.jenkinsci.plugins.xunit.AliasInitializer.addAliases(AliasInitializer.java:47) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at hudson.init.TaskMethodFinder.invoke(TaskMethodFinder.java:105) at hudson.init.TaskMethodFinder$TaskImpl.run(TaskMethodFinder.java:169) at org.jvnet.hudson.reactor.Reactor.runTask(Reactor.java:282) at jenkins.model.Jenkins$7.runTask(Jenkins.java:903) at org.jvnet.hudson.reactor.Reactor$2.run(Reactor.java:210) at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:117) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) at java.lang.Thread.run(Thread.java:745) ``` When writing the `SonarPublisher` config, `JobConfigHistorySaveableListener` is triggered, leading to a `User.current()` call, and ultimately to a Guice error (circular dependency). Since config changes occuring during this startup phase are not linked to any user request, we can avoid the `User.current()` call, and keep the config history entry anonymous. Note this is kind if similar to JENKINS-20988.
jenkinsci · Mar 27, 2015 · 6358945 · 6358945
1 parent c2fb690
commit 6358945
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 3 deletions.
diff --git a/src/main/java/hudson/plugins/jobConfigHistory/JobConfigHistorySaveableListener.java b/src/main/java/hudson/plugins/jobConfigHistory/JobConfigHistorySaveableListener.java
@@ -1,8 +1,10 @@
 package hudson.plugins.jobConfigHistory;
 
+import static hudson.init.InitMilestone.COMPLETED;
 import static java.util.logging.Level.FINEST;
 import hudson.Extension;
 import hudson.XmlFile;
+import hudson.model.Hudson;
 import hudson.model.Saveable;
 import hudson.model.listeners.SaveableListener;
 
@@ -25,7 +27,7 @@ public void onChange(final Saveable o, final XmlFile file) {
         final JobConfigHistory plugin = getPlugin();
         LOG.log(FINEST, "In onChange for {0}", o);
         if (plugin.isSaveable(o, file)) {
-            final HistoryDao configHistoryListenerHelper = getHistoryDao();
+            final HistoryDao configHistoryListenerHelper = getHistoryDao(plugin);
             configHistoryListenerHelper.saveItem(file);
         }
         LOG.log(FINEST, "onChange for {0} done.", o);
@@ -45,8 +47,19 @@ JobConfigHistory getPlugin() {
      *
      * @return helper.
      */
+    @Deprecated
     HistoryDao getHistoryDao() {
-        return PluginUtils.getHistoryDao();
+        return getHistoryDao(PluginUtils.getPlugin());
     }
 
+    /**
+     * Return the helper, making sure its anonymous while Jenkins is still
+     * initializing.
+     * @return helper
+     */
+    HistoryDao getHistoryDao(JobConfigHistory plugin) {
+        return (COMPLETED == Hudson.getInstance().getInitLevel())
+                ? PluginUtils.getHistoryDao(plugin)
+                : PluginUtils.getAnonymousHistoryDao(plugin);
+    }
 }
diff --git a/src/main/java/hudson/plugins/jobConfigHistory/PluginUtils.java b/src/main/java/hudson/plugins/jobConfigHistory/PluginUtils.java
@@ -62,6 +62,16 @@ public static FileHistoryDao getHistoryDao() {
         return getHistoryDao(plugin);
     }
 
+    /**
+     * Like {@link #getHistoryDao()}, but without a user.
+     * Avoids calling {@link User#current()}.
+     * @return historyDao
+     */
+    public static FileHistoryDao getAnonymousHistoryDao() {
+        final JobConfigHistory plugin = getPlugin();
+        return getAnonymousHistoryDao(plugin);
+    }
+
     /**
      * For tests.
      * @param plugin the plugin.
@@ -71,6 +81,16 @@ public static FileHistoryDao getHistoryDao(final JobConfigHistory plugin) {
         return getHistoryDao(plugin, User.current());
     }
 
+    /**
+     * Like {@link #getHistoryDao(JobConfigHistory)}, but without a user.
+     * Avoids calling {@link User#current()}.
+     * @param plugin the plugin.
+     * @return historyDao
+     */
+    public static FileHistoryDao getAnonymousHistoryDao(final JobConfigHistory plugin) {
+        return getHistoryDao(plugin, null);
+    }
+
     static FileHistoryDao getHistoryDao(final JobConfigHistory plugin, final User user) {
         final String maxHistoryEntriesAsString = plugin.getMaxHistoryEntries();
         int maxHistoryEntries = 0;

diff --git a/src/test/java/hudson/plugins/jobConfigHistory/JobConfigHistorySaveableListenerTest.java b/src/test/java/hudson/plugins/jobConfigHistory/JobConfigHistorySaveableListenerTest.java
@@ -45,7 +45,7 @@ JobConfigHistory getPlugin() {
         }
 
         @Override
-        HistoryDao getHistoryDao() {
+        HistoryDao getHistoryDao(JobConfigHistory plugin) {
             return mockedConfigHistoryListenerHelper;
         }
     }