Skip to content

Commit

Permalink
Merge pull request #178 from jglick/parent-JENKINS-47105
Browse files Browse the repository at this point in the history
[JENKINS-47105] Fixing test failure in loaderReleased
  • Loading branch information
svanoort committed Sep 27, 2017
2 parents aa4c4e7 + 53eaaa3 commit bbf99da
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 72 deletions.
9 changes: 8 additions & 1 deletion pom.xml
Expand Up @@ -28,7 +28,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>2.33</version>
<version>2.35</version>
<relativePath />
</parent>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
Expand Down Expand Up @@ -298,6 +298,13 @@
<compatibleSinceVersion>2.18</compatibleSinceVersion>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<reuseForks>false</reuseForks> <!-- TODO reuse seems to cause problems in CpsFlowExecutionMemoryTest -->
</configuration>
</plugin>
</plugins>
</build>
</project>
Expand Up @@ -1036,6 +1036,7 @@ synchronized void onProgramEnd(Outcome outcome) {
}

void cleanUpHeap() {
LOGGER.log(Level.FINE, "cleanUpHeap on {0}", owner);
shell = null;
trusted = null;
if (scriptClass != null) {
Expand All @@ -1045,6 +1046,8 @@ void cleanUpHeap() {
LOGGER.log(Level.WARNING, "failed to clean up memory from " + owner, x);
}
scriptClass = null;
} else {
LOGGER.fine("no scriptClass");
}
// perhaps also set programPromise to null or a precompleted failure?
}
Expand All @@ -1055,6 +1058,7 @@ private static void cleanUpLoader(ClassLoader loader, Set<ClassLoader> encounter
return;
}
if (!(loader instanceof GroovyClassLoader)) {
LOGGER.log(Level.FINER, "ignoring {0}", loader);
return;
}
if (!encounteredLoaders.add(loader)) {
Expand Down
@@ -0,0 +1,109 @@
/*
* The MIT License
*
* Copyright 2017 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 org.jenkinsci.plugins.workflow.cps;

import groovy.lang.MetaClass;
import java.lang.ref.WeakReference;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
import java.util.logging.Level;
import org.codehaus.groovy.reflection.ClassInfo;
import org.codehaus.groovy.transform.ASTTransformationVisitor;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import org.junit.After;
import org.junit.ClassRule;
import org.junit.Test;
import static org.junit.Assert.*;
import org.junit.Ignore;
import org.junit.Rule;
import org.jvnet.hudson.test.BuildWatcher;
import org.jvnet.hudson.test.For;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.LoggerRule;
import org.jvnet.hudson.test.MemoryAssert;

@For(CpsFlowExecution.class) // but kept separate from CpsFlowExecutionTest since it is sensitive to at least a leak from trustedShell()
public class CpsFlowExecutionMemoryTest {

@ClassRule public static BuildWatcher buildWatcher = new BuildWatcher();
@Rule public JenkinsRule r = new JenkinsRule();
@Rule public LoggerRule logger = new LoggerRule().record(CpsFlowExecution.class, Level.FINER);

@After public void clearLoaders() {
LOADERS.clear();
}
private static final List<WeakReference<ClassLoader>> LOADERS = new ArrayList<>();
public static void register(Object o) {
ClassLoader loader = o.getClass().getClassLoader();
System.err.println("registering " + o + " from " + loader);
LOADERS.add(new WeakReference<>(loader));
}

@Test public void loaderReleased() throws Exception {
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
r.jenkins.getWorkspaceFor(p).child("lib.groovy").write(CpsFlowExecutionMemoryTest.class.getName() + ".register(this)", null);
p.setDefinition(new CpsFlowDefinition(CpsFlowExecutionMemoryTest.class.getName() + ".register(this); node {load 'lib.groovy'; evaluate(readFile('lib.groovy'))}", false));
WorkflowRun b = r.assertBuildStatusSuccess(p.scheduleBuild2(0));
assertFalse(((CpsFlowExecution) b.getExecution()).getProgramDataFile().exists());
assertFalse(LOADERS.isEmpty());
{ // TODO it seems that the call to CpsFlowExecutionMemoryTest.register(Object) on a Script1 parameter creates a MetaMethodIndex.Entry.cachedStaticMethod.
// In other words any call to a foundational API might leak classes. Why does Groovy need to do this?
// Unclear whether this is a problem in a realistic environment; for the moment, suppressing it so the test can run with no SoftReference.
MetaClass metaClass = ClassInfo.getClassInfo(CpsFlowExecutionMemoryTest.class).getMetaClass();
Method clearInvocationCaches = metaClass.getClass().getDeclaredMethod("clearInvocationCaches");
clearInvocationCaches.setAccessible(true);
clearInvocationCaches.invoke(metaClass);
}
for (WeakReference<ClassLoader> loaderRef : LOADERS) {
MemoryAssert.assertGC(loaderRef, false);
}
}

@Ignore("creates classes such as script1493642504440203321963 in a new GroovyClassLoader.InnerLoader delegating to CleanGroovyClassLoader which are invisible to cleanUpHeap")
@Test public void doNotUseConfigSlurper() throws Exception {
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition(CpsFlowExecutionMemoryTest.class.getName() + ".register(this); echo(/parsed ${new ConfigSlurper().parse('foo.bar.baz = 99')}/)", false));
WorkflowRun b = r.assertBuildStatusSuccess(p.scheduleBuild2(0));
assertFalse(LOADERS.isEmpty());
try { // as above
Field f = ASTTransformationVisitor.class.getDeclaredField("compUnit");
f.setAccessible(true);
f.set(null, null);
} catch (NoSuchFieldException e) {}
{ // TODO as above
MetaClass metaClass = ClassInfo.getClassInfo(CpsFlowExecutionMemoryTest.class).getMetaClass();
Method clearInvocationCaches = metaClass.getClass().getDeclaredMethod("clearInvocationCaches");
clearInvocationCaches.setAccessible(true);
clearInvocationCaches.invoke(metaClass);
}
for (WeakReference<ClassLoader> loaderRef : LOADERS) {
MemoryAssert.assertGC(loaderRef, true);
}
}

}
Expand Up @@ -30,14 +30,10 @@
import com.gargoylesoftware.htmlunit.WebRequest;
import com.google.common.util.concurrent.ListenableFuture;
import groovy.lang.GroovyShell;
import groovy.lang.MetaClass;
import hudson.AbortException;
import hudson.model.Item;
import hudson.model.Result;
import hudson.model.TaskListener;
import java.lang.ref.WeakReference;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.URL;
Expand All @@ -51,8 +47,6 @@
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import jenkins.model.Jenkins;
import org.codehaus.groovy.reflection.ClassInfo;
import org.codehaus.groovy.transform.ASTTransformationVisitor;
import org.hamcrest.Matchers;
import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException;
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted;
Expand All @@ -66,18 +60,15 @@
import org.jenkinsci.plugins.workflow.support.pickles.SingleTypedPickleFactory;
import org.jenkinsci.plugins.workflow.support.pickles.TryRepeatedly;
import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
import org.junit.After;
import static org.junit.Assert.*;
import org.junit.ClassRule;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runners.model.Statement;
import org.jvnet.hudson.test.BuildWatcher;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.LoggerRule;
import org.jvnet.hudson.test.MemoryAssert;
import org.jvnet.hudson.test.MockAuthorizationStrategy;
import org.jvnet.hudson.test.RestartableJenkinsRule;
import org.jvnet.hudson.test.TestExtension;
Expand All @@ -88,68 +79,6 @@ public class CpsFlowExecutionTest {
@Rule public RestartableJenkinsRule story = new RestartableJenkinsRule();
@Rule public LoggerRule logger = new LoggerRule();

@After public void clearLoaders() {
LOADERS.clear();
}
private static final List<WeakReference<ClassLoader>> LOADERS = new ArrayList<>();
public static void register(Object o) {
ClassLoader loader = o.getClass().getClassLoader();
System.err.println("registering " + o + " from " + loader);
LOADERS.add(new WeakReference<>(loader));
}

@Test public void loaderReleased() {
logger.record(CpsFlowExecution.class, Level.FINER);
story.addStep(new Statement() {
@Override public void evaluate() throws Throwable {
WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
story.j.jenkins.getWorkspaceFor(p).child("lib.groovy").write(CpsFlowExecutionTest.class.getName() + ".register(this)", null);
p.setDefinition(new CpsFlowDefinition(CpsFlowExecutionTest.class.getName() + ".register(this); node {load 'lib.groovy'; evaluate(readFile('lib.groovy'))}", false));
WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0));
assertFalse(((CpsFlowExecution) b.getExecution()).getProgramDataFile().exists());
assertFalse(LOADERS.isEmpty());
{ // TODO it seems that the call to CpsFlowExecutionTest.register(Object) on a Script1 parameter creates a MetaMethodIndex.Entry.cachedStaticMethod.
// In other words any call to a foundational API might leak classes. Why does Groovy need to do this?
// Unclear whether this is a problem in a realistic environment; for the moment, suppressing it so the test can run with no SoftReference.
MetaClass metaClass = ClassInfo.getClassInfo(CpsFlowExecutionTest.class).getMetaClass();
Method clearInvocationCaches = metaClass.getClass().getDeclaredMethod("clearInvocationCaches");
clearInvocationCaches.setAccessible(true);
clearInvocationCaches.invoke(metaClass);
}
for (WeakReference<ClassLoader> loaderRef : LOADERS) {
MemoryAssert.assertGC(loaderRef, false);
}
}
});
}

@Ignore("creates classes such as script1493642504440203321963 in a new GroovyClassLoader.InnerLoader delegating to CleanGroovyClassLoader which are invisible to cleanUpHeap")
@Test public void doNotUseConfigSlurper() throws Exception {
logger.record(CpsFlowExecution.class, Level.FINER);
story.addStep(new Statement() {
@Override public void evaluate() throws Throwable {
WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition(CpsFlowExecutionTest.class.getName() + ".register(this); echo(/parsed ${new ConfigSlurper().parse('foo.bar.baz = 99')}/)", false));
WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0));
assertFalse(LOADERS.isEmpty());
try { // as above
Field f = ASTTransformationVisitor.class.getDeclaredField("compUnit");
f.setAccessible(true);
f.set(null, null);
} catch (NoSuchFieldException e) {}
{ // TODO as above
MetaClass metaClass = ClassInfo.getClassInfo(CpsFlowExecutionTest.class).getMetaClass();
Method clearInvocationCaches = metaClass.getClass().getDeclaredMethod("clearInvocationCaches");
clearInvocationCaches.setAccessible(true);
clearInvocationCaches.invoke(metaClass);
}
for (WeakReference<ClassLoader> loaderRef : LOADERS) {
MemoryAssert.assertGC(loaderRef, true);
}
}
});
}

@Test public void getCurrentExecutions() {
story.addStep(new Statement() {
@Override public void evaluate() throws Throwable {
Expand Down

0 comments on commit bbf99da

Please sign in to comment.