(It could already return null if the run has since been deleted; #1264 failed to mark this properly.)
Reserving IAE for inputs that getExternalizableId could not actually have produced,
and making sure to also throw this in case of non-numeric text after the hash.
Adding Javadoc for both methods.
- The old code would result in multiple triggered updates, with corresponding interrupts of in-progress updates
when faced with lots of slaves coming on line within 1 second of each other.
- New code will ensure that one and only one of the scheduled tasks (i.e. the last one) will trigger the updates.
The original fix was made in 0c3d67097a.
In this fix, I'm also moving the code that creates LeftItem.
It makes no sense that there's Item.cancel(Queue) and Queue.cancel(Item)
and they do different things!
Observed in a test against 1.480:
… hudson.model.Executor run
SEVERE: Unexpected executor death
java.lang.NullPointerException
at hudson.model.Computer.getNode(Computer.java:473)
at hudson.slaves.SlaveComputer.getNode(SlaveComputer.java:177)
at hudson.slaves.SlaveComputer.getRetentionStrategy(SlaveComputer.java:609)
at hudson.slaves.SlaveComputer.taskCompleted(SlaveComputer.java:285)
at hudson.model.queue.WorkUnitContext.synchronizeEnd(WorkUnitContext.java:140)
at hudson.model.Executor.run(Executor.java:253)
As much as we'd like to enable this by default, what we've learned over
the past few days indicate that this is just too disruptive a change. In a nutshell,
- The change is just too big. It touches more than 4000 lines just in the core and remoting,
which makes this by far the biggest change we've done in secret. The chance of something
going wrong is just too high. As recently as yesterday we are still finding significant
problems with this subsystem.
- We've learnt that Subversion 1.x contains Callables that we cannot just whitelist, but
that version is used by roughly a third of users. If we put this on by default, they will
see their builds break.
- Every time we come up with a new way to scan plugins to find impacted code, we always
find several plugins that are affected, indicating that we probably have missed more
plugins that will be impacted.
- We haven't really analyzed past versions of any plugins at all, but as the Subversion
version distribution indicates, considerable portion of people run old plugins.
As we see in the Wiki page (http://jenkins-ci.org/security-144), vast majority of Jenkins
installations will fit in the category of "I have set up Jenkins master and all slaves by
myself", for whom this subsystem is entirely redundant.
Based on the knowledge we now have about the impact of this addition, and considering people
who benefit from this vs people who don't benefit from this, I'm not enabling this feature
out of the box, just yet.
Our plan is to advertise this feature, then get necesary plugin updates in place over time.
At some later point, we can make this feature on by default, which will only impact new
installations. Further down the road, we can inch up the warning given to people who run
without this feature, or remove the GUI configuration and force this feature on, to get
close to the ideal state of having everyone run with this feature on.
Jesse doesn't like this, but I think he understands where I'm coming from. Sorry Jesse,
this time I'm having this my way.
- Added <JOBDIR> based on Daniel's comment
- Path macros need to be translated if on Windows
- Windows treat '/' as a valid path separator, so it needs to be normalized to '\\'.
Daniel Beck pointed out that if we ask people to create files not writable from Jenkins as a means to suppress edit to gui.conf,
then we should just stick to the same approach all the way.
Oliver Gondža