From 85974f3ec5f22522861fe504e36dcd93e185c2dd Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Wed, 22 Apr 2020 12:56:23 +0200 Subject: [PATCH 1/2] [JENKINS-62006] Move getChannelToMaster to a separate class This should avoid classloading issues I've seen, such as Caused by: java.lang.NoClassDefFoundError: javax/servlet/ServletException at hudson.util.ProcessTree.get(ProcessTree.java:432) ... --- .../java/hudson/slaves/SlaveComputer.java | 12 ++------ .../java/hudson/slaves/SlaveComputerUtil.java | 30 +++++++++++++++++++ .../main/java/hudson/util/ProcessTree.java | 8 ++--- 3 files changed, 37 insertions(+), 13 deletions(-) create mode 100644 core/src/main/java/hudson/slaves/SlaveComputerUtil.java diff --git a/core/src/main/java/hudson/slaves/SlaveComputer.java b/core/src/main/java/hudson/slaves/SlaveComputer.java index afd5a81b7d..494a63455e 100644 --- a/core/src/main/java/hudson/slaves/SlaveComputer.java +++ b/core/src/main/java/hudson/slaves/SlaveComputer.java @@ -1067,17 +1067,11 @@ public class SlaveComputer extends Computer { * * @return null if the calling thread doesn't have any trace of where its master is. * @since 1.362 + * @deprecated Use {@link SlaveComputerUtil#getChannelToMaster()} instead. */ + @Deprecated public static VirtualChannel getChannelToMaster() { - if (Jenkins.getInstanceOrNull()!=null) // check if calling thread is on master or on slave - return FilePath.localChannel; - - // if this method is called from within the agent computation thread, this should work - Channel c = Channel.current(); - if (c!=null && Boolean.TRUE.equals(c.getProperty("slave"))) - return c; - - return null; + return SlaveComputerUtil.getChannelToMaster(); } /** diff --git a/core/src/main/java/hudson/slaves/SlaveComputerUtil.java b/core/src/main/java/hudson/slaves/SlaveComputerUtil.java new file mode 100644 index 0000000000..9128a39e33 --- /dev/null +++ b/core/src/main/java/hudson/slaves/SlaveComputerUtil.java @@ -0,0 +1,30 @@ +package hudson.slaves; + +import hudson.FilePath; +import hudson.remoting.Channel; +import hudson.remoting.VirtualChannel; +import jenkins.model.Jenkins; + +public final class SlaveComputerUtil { + private SlaveComputerUtil() {} + /** + * Obtains a {@link VirtualChannel} that allows some computation to be performed on the master. + * This method can be called from any thread on the master, or from agent (more precisely, + * it only works from the remoting request-handling thread in agents, which means if you've started + * separate thread on agents, that'll fail.) + * + * @return null if the calling thread doesn't have any trace of where its master is. + * @since XXX + */ + public static VirtualChannel getChannelToMaster() { + if (Jenkins.getInstanceOrNull()!=null) // check if calling thread is on master or on slave + return FilePath.localChannel; + + // if this method is called from within the agent computation thread, this should work + Channel c = Channel.current(); + if (c!=null && Boolean.TRUE.equals(c.getProperty("slave"))) + return c; + + return null; + } +} diff --git a/core/src/main/java/hudson/util/ProcessTree.java b/core/src/main/java/hudson/util/ProcessTree.java index b12a967a80..919ea9c803 100644 --- a/core/src/main/java/hudson/util/ProcessTree.java +++ b/core/src/main/java/hudson/util/ProcessTree.java @@ -33,7 +33,7 @@ import hudson.FilePath; import hudson.Util; import hudson.remoting.Channel; import hudson.remoting.VirtualChannel; -import hudson.slaves.SlaveComputer; +import hudson.slaves.SlaveComputerUtil; import hudson.util.ProcessKillingVeto.VetoCause; import hudson.util.ProcessTree.OSProcess; import hudson.util.ProcessTreeRemoting.IOSProcess; @@ -193,7 +193,7 @@ public abstract class ProcessTree implements Iterable, IProcessTree, /*package*/ final List getKillers() throws InterruptedException { if (killers==null) try { - VirtualChannel channelToMaster = SlaveComputer.getChannelToMaster(); + VirtualChannel channelToMaster = SlaveComputerUtil.getChannelToMaster(); if (channelToMaster!=null) { killers = channelToMaster.call(new ListAll()); } else { @@ -288,7 +288,7 @@ public abstract class ProcessTree implements Iterable, IProcessTree, // Quick check, does anything exist to check against if (!skipVetoes) { try { - VirtualChannel channelToMaster = SlaveComputer.getChannelToMaster(); + VirtualChannel channelToMaster = SlaveComputerUtil.getChannelToMaster(); if (channelToMaster!=null) { CheckVetoes vetoCheck = new CheckVetoes(this); causeMessage = channelToMaster.call(vetoCheck); @@ -429,7 +429,7 @@ public abstract class ProcessTree implements Iterable, IProcessTree, // Check for the existance of vetoers if I don't know already if (vetoersExist == null) { try { - VirtualChannel channelToMaster = SlaveComputer.getChannelToMaster(); + VirtualChannel channelToMaster = SlaveComputerUtil.getChannelToMaster(); if (channelToMaster != null) { vetoersExist = channelToMaster.call(new DoVetoersExist()); } From 25c8cb6be5f2c14082c0b5619c4136b3d0023816 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Wed, 22 Apr 2020 13:51:20 +0200 Subject: [PATCH 2/2] Fix reviews --- .../java/hudson/slaves/SlaveComputer.java | 5 +- .../java/hudson/slaves/SlaveComputerUtil.java | 30 ---------- .../main/java/hudson/util/ProcessTree.java | 8 +-- .../jenkins/agents/AgentComputerUtil.java | 60 +++++++++++++++++++ 4 files changed, 67 insertions(+), 36 deletions(-) delete mode 100644 core/src/main/java/hudson/slaves/SlaveComputerUtil.java create mode 100644 core/src/main/java/jenkins/agents/AgentComputerUtil.java diff --git a/core/src/main/java/hudson/slaves/SlaveComputer.java b/core/src/main/java/hudson/slaves/SlaveComputer.java index 494a63455e..cc1f5cdaa9 100644 --- a/core/src/main/java/hudson/slaves/SlaveComputer.java +++ b/core/src/main/java/hudson/slaves/SlaveComputer.java @@ -54,6 +54,7 @@ import hudson.util.StreamTaskListener; import hudson.util.VersionNumber; import hudson.util.io.RewindableFileOutputStream; import hudson.util.io.RewindableRotatingFileOutputStream; +import jenkins.agents.AgentComputerUtil; import jenkins.model.Jenkins; import jenkins.security.ChannelConfigurator; import jenkins.security.MasterToSlaveCallable; @@ -1067,11 +1068,11 @@ public class SlaveComputer extends Computer { * * @return null if the calling thread doesn't have any trace of where its master is. * @since 1.362 - * @deprecated Use {@link SlaveComputerUtil#getChannelToMaster()} instead. + * @deprecated Use {@link AgentComputerUtil#getChannelToMaster()} instead. */ @Deprecated public static VirtualChannel getChannelToMaster() { - return SlaveComputerUtil.getChannelToMaster(); + return AgentComputerUtil.getChannelToMaster(); } /** diff --git a/core/src/main/java/hudson/slaves/SlaveComputerUtil.java b/core/src/main/java/hudson/slaves/SlaveComputerUtil.java deleted file mode 100644 index 9128a39e33..0000000000 --- a/core/src/main/java/hudson/slaves/SlaveComputerUtil.java +++ /dev/null @@ -1,30 +0,0 @@ -package hudson.slaves; - -import hudson.FilePath; -import hudson.remoting.Channel; -import hudson.remoting.VirtualChannel; -import jenkins.model.Jenkins; - -public final class SlaveComputerUtil { - private SlaveComputerUtil() {} - /** - * Obtains a {@link VirtualChannel} that allows some computation to be performed on the master. - * This method can be called from any thread on the master, or from agent (more precisely, - * it only works from the remoting request-handling thread in agents, which means if you've started - * separate thread on agents, that'll fail.) - * - * @return null if the calling thread doesn't have any trace of where its master is. - * @since XXX - */ - public static VirtualChannel getChannelToMaster() { - if (Jenkins.getInstanceOrNull()!=null) // check if calling thread is on master or on slave - return FilePath.localChannel; - - // if this method is called from within the agent computation thread, this should work - Channel c = Channel.current(); - if (c!=null && Boolean.TRUE.equals(c.getProperty("slave"))) - return c; - - return null; - } -} diff --git a/core/src/main/java/hudson/util/ProcessTree.java b/core/src/main/java/hudson/util/ProcessTree.java index 919ea9c803..8e7bcb7e6c 100644 --- a/core/src/main/java/hudson/util/ProcessTree.java +++ b/core/src/main/java/hudson/util/ProcessTree.java @@ -33,7 +33,7 @@ import hudson.FilePath; import hudson.Util; import hudson.remoting.Channel; import hudson.remoting.VirtualChannel; -import hudson.slaves.SlaveComputerUtil; +import jenkins.agents.AgentComputerUtil; import hudson.util.ProcessKillingVeto.VetoCause; import hudson.util.ProcessTree.OSProcess; import hudson.util.ProcessTreeRemoting.IOSProcess; @@ -193,7 +193,7 @@ public abstract class ProcessTree implements Iterable, IProcessTree, /*package*/ final List getKillers() throws InterruptedException { if (killers==null) try { - VirtualChannel channelToMaster = SlaveComputerUtil.getChannelToMaster(); + VirtualChannel channelToMaster = AgentComputerUtil.getChannelToMaster(); if (channelToMaster!=null) { killers = channelToMaster.call(new ListAll()); } else { @@ -288,7 +288,7 @@ public abstract class ProcessTree implements Iterable, IProcessTree, // Quick check, does anything exist to check against if (!skipVetoes) { try { - VirtualChannel channelToMaster = SlaveComputerUtil.getChannelToMaster(); + VirtualChannel channelToMaster = AgentComputerUtil.getChannelToMaster(); if (channelToMaster!=null) { CheckVetoes vetoCheck = new CheckVetoes(this); causeMessage = channelToMaster.call(vetoCheck); @@ -429,7 +429,7 @@ public abstract class ProcessTree implements Iterable, IProcessTree, // Check for the existance of vetoers if I don't know already if (vetoersExist == null) { try { - VirtualChannel channelToMaster = SlaveComputerUtil.getChannelToMaster(); + VirtualChannel channelToMaster = AgentComputerUtil.getChannelToMaster(); if (channelToMaster != null) { vetoersExist = channelToMaster.call(new DoVetoersExist()); } diff --git a/core/src/main/java/jenkins/agents/AgentComputerUtil.java b/core/src/main/java/jenkins/agents/AgentComputerUtil.java new file mode 100644 index 0000000000..3aa026124e --- /dev/null +++ b/core/src/main/java/jenkins/agents/AgentComputerUtil.java @@ -0,0 +1,60 @@ +/* + * The MIT License + * + * Copyright 2020 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package jenkins.agents; + +import edu.umd.cs.findbugs.annotations.CheckForNull; +import hudson.FilePath; +import hudson.remoting.Channel; +import hudson.remoting.VirtualChannel; +import jenkins.model.Jenkins; + +public final class AgentComputerUtil { + private AgentComputerUtil() { + } + + /** + * Obtains a {@link VirtualChannel} that allows some computation to be performed on the master. + * This method can be called from any thread on the master, or from agent (more precisely, + * it only works from the remoting request-handling thread in agents, which means if you've started + * separate thread on agents, that'll fail.) + * + * @return null if the calling thread doesn't have any trace of where its master is. + * @since TODO + */ + @CheckForNull + public static VirtualChannel getChannelToMaster() { + if (Jenkins.getInstanceOrNull() != null) {// check if calling thread is on master or on agent + return FilePath.localChannel; + } + + // if this method is called from within the agent computation thread, this should work + Channel c = Channel.current(); + if (c != null && Boolean.TRUE.equals(c.getProperty("slave"))) { + return c; + } + + return null; + } +}