From f5a7869506fdf8c53b09417a61a6e71feb539450 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 20 Sep 2012 13:38:36 -0400 Subject: [PATCH] Null safety of Trigger.timer. A botched startup (say by an error in InitializerFinder) can cause log to be filled with junk like: Sep 20, 2012 1:10:50 PM hudson.ExtensionFinder$GuiceFinder instantiate WARNING: Failed to load hudson.node_monitors.DiskSpaceMonitor java.lang.InstantiationException: java.lang.ExceptionInInitializerError at net.java.sezpoz.IndexItem.instance(IndexItem.java:193) at hudson.ExtensionFinder$GuiceFinder.instantiate(ExtensionFinder.java:353) at hudson.ExtensionFinder$GuiceFinder.access$400(ExtensionFinder.java:232) at hudson.ExtensionFinder$GuiceFinder$SezpozModule$1.get(ExtensionFinder.java:509) at com.google.inject.internal.ProviderInternalFactory.provision(ProviderInternalFactory.java:84) at com.google.inject.internal.InternalFactoryToInitializableAdapter.provision(InternalFactoryToInitializableAdapter.java:52) at com.google.inject.internal.ProviderInternalFactory.circularGet(ProviderInternalFactory.java:66) at com.google.inject.internal.InternalFactoryToInitializableAdapter.get(InternalFactoryToInitializableAdapter.java:45) at com.google.inject.internal.ProviderToInternalFactoryAdapter$1.call(ProviderToInternalFactoryAdapter.java:46) at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1018) at com.google.inject.internal.ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:40) at com.google.inject.Scopes$1$1.get(Scopes.java:59) at hudson.ExtensionFinder$GuiceFinder$4$1.get(ExtensionFinder.java:420) at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:41) at com.google.inject.internal.InjectorImpl$3$1.call(InjectorImpl.java:965) at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1011) at com.google.inject.internal.InjectorImpl$3.get(InjectorImpl.java:961) at hudson.ExtensionFinder$GuiceFinder._find(ExtensionFinder.java:389) at hudson.ExtensionFinder$GuiceFinder.find(ExtensionFinder.java:380) at hudson.ExtensionFinder._find(ExtensionFinder.java:149) at hudson.ClassicPluginStrategy.findComponents(ClassicPluginStrategy.java:289) at hudson.ExtensionList.load(ExtensionList.java:278) at hudson.ExtensionList.ensureLoaded(ExtensionList.java:231) at hudson.ExtensionList.getComponents(ExtensionList.java:149) at hudson.DescriptorExtensionList.load(DescriptorExtensionList.java:182) at hudson.ExtensionList.ensureLoaded(ExtensionList.java:231) at hudson.ExtensionList.size(ExtensionList.java:157) at java.util.AbstractCollection.isEmpty(AbstractCollection.java:86) at hudson.model.labels.LabelAtom.updateTransientActions(LabelAtom.java:107) at hudson.model.labels.LabelAtom.load(LabelAtom.java:189) at jenkins.model.Jenkins.getLabelAtom(Jenkins.java:1545) at jenkins.model.Jenkins.getSelfLabel(Jenkins.java:2422) at hudson.model.Node.getAssignedLabels(Node.java:240) at jenkins.model.Jenkins$17.run(Jenkins.java:2499) at org.jvnet.hudson.reactor.TaskGraphBuilder$TaskImpl.run(TaskGraphBuilder.java:146) at org.jvnet.hudson.reactor.Reactor.runTask(Reactor.java:259) at jenkins.model.Jenkins$7.runTask(Jenkins.java:875) at org.jvnet.hudson.reactor.Reactor$2.run(Reactor.java:187) at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:94) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603) at java.lang.Thread.run(Thread.java:722) Caused by: java.lang.ExceptionInInitializerError 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:601) at net.java.sezpoz.IndexItem.instance(IndexItem.java:183) ... 41 more Caused by: java.lang.NullPointerException at hudson.node_monitors.AbstractNodeMonitorDescriptor.schedule(AbstractNodeMonitorDescriptor.java:73) at hudson.node_monitors.AbstractNodeMonitorDescriptor.(AbstractNodeMonitorDescriptor.java:58) at hudson.node_monitors.AbstractNodeMonitorDescriptor.(AbstractNodeMonitorDescriptor.java:54) at hudson.node_monitors.DiskSpaceMonitorDescriptor.(DiskSpaceMonitorDescriptor.java:49) at hudson.node_monitors.DiskSpaceMonitor$1.(DiskSpaceMonitor.java:62) at hudson.node_monitors.DiskSpaceMonitor.(DiskSpaceMonitor.java:62) ... 46 more --- .../main/java/hudson/model/AperiodicWork.java | 6 +++- core/src/main/java/hudson/model/Queue.java | 9 +++-- .../AbstractNodeMonitorDescriptor.java | 14 +++++--- .../main/java/hudson/triggers/Trigger.java | 36 +++++++++++-------- .../java/hudson/util/DoubleLaunchChecker.java | 14 +++++--- core/src/main/java/jenkins/model/Jenkins.java | 26 +++++++++----- 6 files changed, 69 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/hudson/model/AperiodicWork.java b/core/src/main/java/hudson/model/AperiodicWork.java index b46ac8fc0e..512db0efc0 100644 --- a/core/src/main/java/hudson/model/AperiodicWork.java +++ b/core/src/main/java/hudson/model/AperiodicWork.java @@ -30,6 +30,7 @@ import hudson.triggers.Trigger; import jenkins.model.Jenkins; import java.util.Random; +import java.util.Timer; import java.util.logging.Logger; @@ -84,7 +85,10 @@ public abstract class AperiodicWork extends SafeTimerTask implements ExtensionPo @Override public final void doRun() throws Exception{ doAperiodicRun(); - Trigger.timer.schedule(getNewInstance(), getRecurrencePeriod()); + Timer timer = Trigger.timer; + if (timer != null) { + timer.schedule(getNewInstance(), getRecurrencePeriod()); + } } protected abstract void doAperiodicRun(); diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java index 2fde52bfd8..e7e0142bc1 100644 --- a/core/src/main/java/hudson/model/Queue.java +++ b/core/src/main/java/hudson/model/Queue.java @@ -85,6 +85,7 @@ import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Set; +import java.util.Timer; import java.util.TreeSet; import java.util.Map.Entry; import java.util.concurrent.atomic.AtomicInteger; @@ -93,7 +94,6 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Level; import java.util.logging.Logger; -import javax.management.timer.Timer; import javax.servlet.ServletException; import jenkins.model.Jenkins; @@ -1746,8 +1746,11 @@ public class Queue extends ResourceController implements Saveable { MaintainTask(Queue queue) { this.queue = new WeakReference(queue); - long interval = 5 * Timer.ONE_SECOND; - Trigger.timer.schedule(this, interval, interval); + long interval = 5000; + Timer timer = Trigger.timer; + if (timer != null) { + timer.schedule(this, interval, interval); + } } protected void doRun() { diff --git a/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java b/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java index 4d20fb2c9e..0db9f64c2b 100644 --- a/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java +++ b/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java @@ -36,6 +36,7 @@ import java.io.IOException; import java.util.Date; import java.util.HashMap; import java.util.Map; +import java.util.Timer; import java.util.logging.Level; import java.util.logging.Logger; @@ -70,11 +71,14 @@ public abstract class AbstractNodeMonitorDescriptor extends Descriptor implements Describable> * * If plugins want to run periodic jobs, they should implement {@link PeriodicWork}. */ - public static Timer timer; + @SuppressWarnings("MS_SHOULD_BE_FINAL") + public static @CheckForNull Timer timer; @Initializer(after=JOB_LOADED) public static void init() { new DoubleLaunchChecker().schedule(); - // start all PeridocWorks - for(PeriodicWork p : PeriodicWork.all()) - timer.scheduleAtFixedRate(p,p.getInitialDelay(),p.getRecurrencePeriod()); - - // start all AperidocWorks - for(AperiodicWork p : AperiodicWork.all()) - timer.schedule(p,p.getInitialDelay()); - - // start monitoring nodes, although there's no hurry. - timer.schedule(new SafeTimerTask() { - public void doRun() { - ComputerSet.initialize(); + Timer _timer = timer; + if (_timer != null) { + // start all PeridocWorks + for(PeriodicWork p : PeriodicWork.all()) { + _timer.scheduleAtFixedRate(p,p.getInitialDelay(),p.getRecurrencePeriod()); } - }, 1000*10); + + // start all AperidocWorks + for(AperiodicWork p : AperiodicWork.all()) { + _timer.schedule(p,p.getInitialDelay()); + } + + // start monitoring nodes, although there's no hurry. + _timer.schedule(new SafeTimerTask() { + public void doRun() { + ComputerSet.initialize(); + } + }, 1000*10); + } } /** diff --git a/core/src/main/java/hudson/util/DoubleLaunchChecker.java b/core/src/main/java/hudson/util/DoubleLaunchChecker.java index bcdb07c74a..3abf89b2ee 100644 --- a/core/src/main/java/hudson/util/DoubleLaunchChecker.java +++ b/core/src/main/java/hudson/util/DoubleLaunchChecker.java @@ -40,6 +40,7 @@ import java.util.logging.Logger; import java.util.logging.Level; import java.lang.management.ManagementFactory; import java.lang.reflect.Method; +import java.util.Timer; /** * Makes sure that no other Hudson uses our JENKINS_HOME directory, @@ -145,11 +146,14 @@ public class DoubleLaunchChecker { public void schedule() { // randomize the scheduling so that multiple Hudson instances will write at the file at different time long MINUTE = 1000*60; - Trigger.timer.schedule(new SafeTimerTask() { - protected void doRun() { - execute(); - } - },(random.nextInt(30)+60)*MINUTE); + Timer timer = Trigger.timer; + if (timer != null) { + timer.schedule(new SafeTimerTask() { + protected void doRun() { + execute(); + } + },(random.nextInt(30)+60)*MINUTE); + } } /** diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index d3dd6a3a2a..360f52dba2 100755 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -727,7 +727,10 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro * @param pluginManager * If non-null, use existing plugin manager. create a new one. */ - @edu.umd.cs.findbugs.annotations.SuppressWarnings("SC_START_IN_CTOR") // bug in FindBugs. It flags UDPBroadcastThread.start() call but that's for another class + @edu.umd.cs.findbugs.annotations.SuppressWarnings({ + "SC_START_IN_CTOR", // bug in FindBugs. It flags UDPBroadcastThread.start() call but that's for another class + "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD" // Trigger.timer + }) protected Jenkins(File root, ServletContext context, PluginManager pluginManager) throws IOException, InterruptedException, ReactorException { long start = System.currentTimeMillis(); @@ -816,12 +819,15 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro } dnsMultiCast = new DNSMultiCast(this); - Trigger.timer.scheduleAtFixedRate(new SafeTimerTask() { - @Override - protected void doRun() throws Exception { - trimLabels(); - } - }, TimeUnit2.MINUTES.toMillis(5), TimeUnit2.MINUTES.toMillis(5)); + Timer timer = Trigger.timer; + if (timer != null) { + timer.scheduleAtFixedRate(new SafeTimerTask() { + @Override + protected void doRun() throws Exception { + trimLabels(); + } + }, TimeUnit2.MINUTES.toMillis(5), TimeUnit2.MINUTES.toMillis(5)); + } updateComputerList(); @@ -2563,6 +2569,7 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro /** * Called to shut down the system. */ + @edu.umd.cs.findbugs.annotations.SuppressWarnings("ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD") public void cleanUp() { for (ItemListener l : ItemListener.all()) l.onBeforeShutdown(); @@ -2579,7 +2586,10 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro if(dnsMultiCast!=null) dnsMultiCast.close(); interruptReloadThread(); - Trigger.timer.cancel(); + Timer timer = Trigger.timer; + if (timer != null) { + timer.cancel(); + } // TODO: how to wait for the completion of the last job? Trigger.timer = null; if(tcpSlaveAgentListener!=null) -- GitLab