From abf9fd36582ce6277dec929e89f078678e587209 Mon Sep 17 00:00:00 2001 From: Stephen Connolly Date: Tue, 8 Mar 2016 12:11:38 +0000 Subject: [PATCH 1/4] [FIXED JENKINS-33384] clean up the PluginServletFilter instances on Jenkins.cleanUp() --- .../java/hudson/util/PluginServletFilter.java | 22 +++++++++++++++++++ core/src/main/java/jenkins/model/Jenkins.java | 3 +++ 2 files changed, 25 insertions(+) diff --git a/core/src/main/java/hudson/util/PluginServletFilter.java b/core/src/main/java/hudson/util/PluginServletFilter.java index 9e20ea6b50..06fd67a925 100644 --- a/core/src/main/java/hudson/util/PluginServletFilter.java +++ b/core/src/main/java/hudson/util/PluginServletFilter.java @@ -42,6 +42,8 @@ import java.util.Vector; import java.util.concurrent.CopyOnWriteArrayList; import java.util.logging.Level; import java.util.logging.Logger; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Servlet {@link Filter} that chains multiple {@link Filter}s, provided by plugins @@ -142,5 +144,25 @@ public class PluginServletFilter implements Filter, ExtensionPoint { list.clear(); } + @Restricted(NoExternalUse.class) + public static void cleanUp() { + PluginServletFilter instance = getInstance(Jenkins.getInstance().servletContext); + if (instance != null) { + for (Iterator iterator = instance.list.iterator(); iterator.hasNext(); ) { + Filter f = iterator.next(); + try { + f.destroy(); + } catch (RuntimeException e) { + LOGGER.log(Level.WARNING, "Filter " + f + " propagated an exception from its destroy method", e); + } catch (Error e) { + throw e; // we are not supposed to catch errors, don't log as could be an OOM + } catch (Throwable e) { + LOGGER.log(Level.SEVERE, "Filter " + f + " propagated an exception from its destroy method", e); + } + iterator.remove(); + } + } + } + private static final Logger LOGGER = Logger.getLogger(PluginServletFilter.class.getName()); } diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index b8d83bd141..19d22979d8 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -178,6 +178,7 @@ import hudson.util.JenkinsReloadFailed; import hudson.util.Memoizer; import hudson.util.MultipartFormDataParser; import hudson.util.NamingThreadFactory; +import hudson.util.PluginServletFilter; import hudson.util.RemotingDiagnostics; import hudson.util.RemotingDiagnostics.HeapDump; import hudson.util.TextFile; @@ -2901,6 +2902,8 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve LOGGER.log(Level.WARNING, "Failed to shut down properly",e); } + PluginServletFilter.cleanUp(); + LogFactory.releaseAll(); theInstance = null; From f68f5fb24f0fe69b45bf7286b222f448d26b2fac Mon Sep 17 00:00:00 2001 From: Stephen Connolly Date: Tue, 8 Mar 2016 23:05:00 +0000 Subject: [PATCH 2/4] [FIXED JENKINS-33414] Added an API to allow identification of the Jenkins JVM - Fix the ProxyConfiguration code to not require Jenkins class to be loaded in remote JVMs --- .../main/java/hudson/ProxyConfiguration.java | 35 +++++++-- core/src/main/java/hudson/WebAppMain.java | 35 ++++++--- .../main/java/jenkins/util/JenkinsJVM.java | 73 +++++++++++++++++++ .../java/jenkins/util/JenkinsJVMTest.java | 36 +++++++++ 4 files changed, 159 insertions(+), 20 deletions(-) create mode 100644 core/src/main/java/jenkins/util/JenkinsJVM.java create mode 100644 core/src/test/java/jenkins/util/JenkinsJVMTest.java diff --git a/core/src/main/java/hudson/ProxyConfiguration.java b/core/src/main/java/hudson/ProxyConfiguration.java index 9b51990925..e3c42b09f5 100644 --- a/core/src/main/java/hudson/ProxyConfiguration.java +++ b/core/src/main/java/hudson/ProxyConfiguration.java @@ -47,7 +47,9 @@ import java.net.URLConnection; import java.util.Collections; import java.util.List; import java.util.regex.Pattern; +import javax.annotation.CheckForNull; import jenkins.model.Jenkins; +import jenkins.util.JenkinsJVM; import org.apache.commons.httpclient.Credentials; import org.apache.commons.httpclient.HttpClient; import org.apache.commons.httpclient.UsernamePasswordCredentials; @@ -215,8 +217,7 @@ public final class ProxyConfiguration extends AbstractDescribableImpl Date: Tue, 8 Mar 2016 23:15:13 +0000 Subject: [PATCH 3/4] [JENKINS-33414] Fix Javadoc comments --- core/src/main/java/jenkins/util/JenkinsJVM.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/jenkins/util/JenkinsJVM.java b/core/src/main/java/jenkins/util/JenkinsJVM.java index 6479e9b70d..dce80915f3 100644 --- a/core/src/main/java/jenkins/util/JenkinsJVM.java +++ b/core/src/main/java/jenkins/util/JenkinsJVM.java @@ -39,7 +39,7 @@ public class JenkinsJVM { * Verify that the classloader that loaded this class is the classloader from which {@link Jenkins} has been * started. * - * @throws IllegalArgumentException if this is not the classloader on the JVM that started {@link Jenkins}. + * @throws IllegalStateException if this is not the classloader on the JVM that started {@link Jenkins}. */ public static void checkJenkinsJVM() { if (!isJenkinsJVM()) { @@ -51,7 +51,7 @@ public class JenkinsJVM { * Verify that the classloader that loaded this class is not the classloader from which {@link Jenkins} has been * started. * - * @throws IllegalArgumentException if this is the classloader on the JVM that started {@link Jenkins}. + * @throws IllegalStateException if this is the classloader on the JVM that started {@link Jenkins}. */ public static void checkNotJenkinsJVM() { if (isJenkinsJVM()) { From 58cd6f0ab1048f01642947e727ea8d49c5b82ac6 Mon Sep 17 00:00:00 2001 From: Stephen Connolly Date: Tue, 8 Mar 2016 23:31:38 +0000 Subject: [PATCH 4/4] [JENKINS-33414] Add a test case that verifies for real --- core/src/main/java/jenkins/model/Jenkins.java | 8 +++++ .../java/jenkins/util/JenkinsJVMRealTest.java | 34 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 test/src/test/java/jenkins/util/JenkinsJVMRealTest.java diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 7fcf37c7ca..93c2900360 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -198,6 +198,7 @@ import jenkins.security.ConfidentialStore; import jenkins.security.SecurityListener; import jenkins.security.MasterToSlaveCallable; import jenkins.slaves.WorkspaceLocator; +import jenkins.util.JenkinsJVM; import jenkins.util.Timer; import jenkins.util.io.FileBoolean; import net.sf.json.JSONObject; @@ -750,6 +751,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD" // Trigger.timer }) protected Jenkins(File root, ServletContext context, PluginManager pluginManager) throws IOException, InterruptedException, ReactorException { + JenkinsJVMAccess._setJenkinsJVM(true); // set it for unit tests as they will not have gone through WebAppMain long start = System.currentTimeMillis(); // As Jenkins is starting, grant this process full control @@ -4334,4 +4336,10 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve } } + private static final class JenkinsJVMAccess extends JenkinsJVM { + private static void _setJenkinsJVM(boolean jenkinsJVM) { + JenkinsJVM.setJenkinsJVM(jenkinsJVM); + } + } + } diff --git a/test/src/test/java/jenkins/util/JenkinsJVMRealTest.java b/test/src/test/java/jenkins/util/JenkinsJVMRealTest.java new file mode 100644 index 0000000000..bdce8a15e3 --- /dev/null +++ b/test/src/test/java/jenkins/util/JenkinsJVMRealTest.java @@ -0,0 +1,34 @@ +package jenkins.util; + +import hudson.slaves.DumbSlave; +import java.io.IOException; +import jenkins.security.MasterToSlaveCallable; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +public class JenkinsJVMRealTest { + + @ClassRule + public static JenkinsRule j = new JenkinsRule(); + + @Test + public void isJenkinsJVM() throws Exception { + assertThat(new IsJenkinsJVM().call(), is(true)); + DumbSlave slave = j.createOnlineSlave(); + assertThat(slave.getChannel().call(new IsJenkinsJVM()), is(false)); + } + + public static class IsJenkinsJVM extends MasterToSlaveCallable { + + @Override + public Boolean call() throws IOException { + return JenkinsJVM.isJenkinsJVM(); + } + } + +}