Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<dependency>
<groupId>se.jiderhamn</groupId>
<artifactId>classloader-leak-test-framework</artifactId>
<version>1.1.1</version>
<version>1.1.2-SNAPSHOT</version>
<scope>test</scope>
</dependency>
<!-- Required by some of the tested APIs -->
Expand Down Expand Up @@ -87,7 +87,7 @@
<dependency>
<groupId>org.geotools</groupId>
<artifactId>gt-metadata</artifactId>
<version>2.6.2</version>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actualy working for you, @davoustp ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is.
But the gt-metadata artifact has moved to another repo, and not available in public Maven: http://maven.geo-solutions.it/
I struggled quite a bit to find this one, to be honest. :-)

<version>26.1.06</version>
<scope>test</scope>
</dependency>
<!-- Test leak in JSF api -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.Collection;
import java.util.Timer;
import java.util.TimerTask;

import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -21,7 +22,16 @@ public class StopThreadsCleanUp_TimerTest {
*/
@Test
public void createTimer() throws IllegalAccessException, NoSuchFieldException {
new Timer("MyTimer"); // Create new Timer to spawn new TimerThread
Timer timer =new Timer("MyTimer"); // Create new Timer to spawn new TimerThread
// Ensure that a task is scheduled against this timer, making sure that scheduled tasks are cancelled by StopThreadsCleanUp.
// Not doing so will also cause this test to intermittently fail with JDK17+ with
// "ClassLoader has been garbage collected, while test is expected to leak"
// as it now uses a new JVM-internal jdk.internal.ref.Cleaner instead of the previously-used finalizer mechanism.
// See https://github.com/openjdk/jdk/blob/jdk-17+35/src/java.base/share/classes/java/util/Timer.java
timer.schedule(new TimerTask() {
@Override
public void run() {}
},10000);
Thread.yield(); // Allow the Timer thread to start
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,28 @@
package se.jiderhamn.classloader.leak.prevention.cleanup;

import javax.el.*;
import javax.el.BeanELResolver;
import javax.el.ELContext;
import javax.el.ELResolver;
import javax.el.FunctionMapper;
import javax.el.VariableMapper;

import org.junit.Before;

import se.jiderhamn.classloader.leak.prevention.ClassLoaderLeakPreventor;
import se.jiderhamn.classloader.leak.prevention.ClassLoaderPreMortemCleanUp;
import se.jiderhamn.classloader.leak.prevention.cleanup.BeanELResolverCleanUpTest.BeanELResolverCombinedCleanUp;

/**
* Test case for {@link BeanELResolverCleanUp}
*
* NOTE: this case also triggers the leak from com.sun.beans.introspect.ClassInfo.CACHE,
* which should be handled as part of {@link BeanIntrospectorCleanUp}.
* See https://github.com/mjiderhamn/classloader-leak-prevention/issues/123 for the specifics.
* A combined (JavaServerFaces2746CleanUp,BeanIntrospectorCleanUp) cleanup is used here to handle this test
*
* @author Mattias Jiderhamn
*/
public class BeanELResolverCleanUpTest extends ClassLoaderPreMortemCleanUpTestBase<BeanELResolverCleanUp> {
public class BeanELResolverCleanUpTest extends ClassLoaderPreMortemCleanUpTestBase<BeanELResolverCombinedCleanUp> {

@Before
public void setUp() {
Expand Down Expand Up @@ -56,5 +70,13 @@ public VariableMapper getVariableMapper() {
throw new UnsupportedOperationException("dummy");
}
}


public static class BeanELResolverCombinedCleanUp implements ClassLoaderPreMortemCleanUp {
@Override
public void cleanUp(ClassLoaderLeakPreventor preventor) {
new BeanELResolverCleanUp().cleanUp(preventor);
new BeanIntrospectorCleanUp().cleanUp(preventor);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,21 @@

import com.sun.faces.el.ELContextImpl;

import se.jiderhamn.classloader.leak.prevention.ClassLoaderLeakPreventor;
import se.jiderhamn.classloader.leak.prevention.ClassLoaderPreMortemCleanUp;
import se.jiderhamn.classloader.leak.prevention.cleanup.JavaServerFaces2746CleanUpTest.JavaServerFaces2746CombinedCleanup;

/**
* Test case for {@link JavaServerFaces2746CleanUp}
*
* NOTE: this case also triggers the leak from com.sun.beans.introspect.ClassInfo.CACHE,
* which should be handled as part of {@link BeanIntrospectorCleanUp}.
* See https://github.com/mjiderhamn/classloader-leak-prevention/issues/123 for the specifics.
* A combined (JavaServerFaces2746CleanUp,BeanIntrospectorCleanUp) cleanup is used here to handle this test
*
* @author Mattias Jiderhamn
*/
public class JavaServerFaces2746CleanUpTest extends ClassLoaderPreMortemCleanUpTestBase<JavaServerFaces2746CleanUp> {
public class JavaServerFaces2746CleanUpTest extends ClassLoaderPreMortemCleanUpTestBase<JavaServerFaces2746CombinedCleanup> {

/**
* Trigger leak by explicit call to {@link BeanELResolver#getFeatureDescriptors(javax.el.ELContext, Object)}.
Expand Down Expand Up @@ -63,4 +73,12 @@ protected void triggerLeak() throws Exception {

new ThreadGroupContextCleanUp().cleanUp(getClassLoaderLeakPreventor());
}

public static class JavaServerFaces2746CombinedCleanup implements ClassLoaderPreMortemCleanUp {
@Override
public void cleanUp(ClassLoaderLeakPreventor preventor) {
new JavaServerFaces2746CleanUp().cleanUp(preventor);
new BeanIntrospectorCleanUp().cleanUp(preventor);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
package se.jiderhamn.classloader.leak.prevention.cleanup;

import org.junit.Assume;

import se.jiderhamn.classloader.leak.prevention.support.JavaVersion;

/**
* Test cases for {@link JavaUtilLoggingLevelCleanUp}
* @author Mattias Jiderhamn
*/
public class JavaUtilLoggingLevelCleanUpTest extends ClassLoaderPreMortemCleanUpTestBase<JavaUtilLoggingLevelCleanUp> {
@Override
protected void triggerLeak() throws Exception {
// Leak does not occur any more with JDK11+
Assume.assumeTrue(JavaVersion.IS_JAVA_10_OR_EARLIER);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't triggerLeakWithoutCleanup() overridden in this case?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question - I may have overlooked this one...


// TODO Log
// Logger logger = Logger.getLogger(JavaUtilLoggingLevelCleanUpTest.class.getName());
// logger.setLevel(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
import se.jiderhamn.classloader.leak.prevention.ClassLoaderPreMortemCleanUp;
import se.jiderhamn.classloader.leak.prevention.ReplaceDOMNormalizerSerializerAbortException;
import se.jiderhamn.classloader.leak.prevention.preinit.ReplaceDOMNormalizerSerializerAbortExceptionInitiatorTest;
import se.jiderhamn.classloader.leak.prevention.support.JavaVersion;

import org.junit.Assume;

/**
* Test cases for {@link ReplaceDOMNormalizerSerializerAbortException} when used as {@link ClassLoaderPreMortemCleanUp}
Expand All @@ -13,4 +16,12 @@ public class ReplaceDOMNormalizerSerializerAbortExceptionCleanUpTest extends Cla
public void triggerLeak() throws Exception {
ReplaceDOMNormalizerSerializerAbortExceptionInitiatorTest.triggerLeak();
}

@Override
public void triggerLeakWithoutCleanup() throws Exception {
// Leak does not occur any more with JDK8+
Assume.assumeTrue(JavaVersion.IS_JAVA_1_6 || JavaVersion.IS_JAVA_1_7);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it have made more sense to make the original triggerLeakWithoutCleanup() do something like Assume.assumeTrue(isRelevantForJavaVersion()) with a default implementation of isRelevantForJavaVersion() that returns true, and then override that method in the respective tests? Feels like it would be more readable?

Plus, isRelevantForJavaVersion() could be named the same for both CleanUp tests and PreInitiator tests.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed.

super.triggerLeakWithoutCleanup();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
import java.io.File;
import javax.net.ssl.SSLContext;

import org.junit.Assume;

import se.jiderhamn.classloader.leak.prevention.support.JavaVersion;

/**
* Test case for {@link X509TrustManagerImplUnparseableExtensionCleanUp}
* @author Mattias Jiderhamn
Expand All @@ -14,4 +18,12 @@ protected void triggerLeak() throws Exception {
System.setProperty("javax.net.ssl.trustStore", keystore.getAbsolutePath());
SSLContext.getDefault();
}

@Override
public void triggerLeakWithoutCleanup() throws Exception {
// Leak does not occur any more with JDK17+ (and maybe some versions above JDK11 - not tested)
Assume.assumeTrue(JavaVersion.IS_JAVA_16_OR_EARLIER);
super.triggerLeakWithoutCleanup();
}

}
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package se.jiderhamn.classloader.leak.prevention.preinit;

import org.junit.Assume;
import org.junit.Before;

import se.jiderhamn.classloader.leak.prevention.support.JavaVersion;

/**
* Test cases for {@link LdapPoolManagerInitiator}
* @author Mattias Jiderhamn
Expand All @@ -11,4 +14,11 @@ public class LdapPoolManagerInitiatorTest extends PreClassLoaderInitiatorTestBas
public void setSystemProperty() {
System.setProperty("com.sun.jndi.ldap.connect.pool.timeout", "1"); // Required to trigger leak
}

@Override
public void firstShouldLeak() throws Exception {
// Leak does not occur any more with JDK8+
Assume.assumeTrue(JavaVersion.IS_JAVA_1_6 || JavaVersion.IS_JAVA_1_7);
super.firstShouldLeak();
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,20 @@
package se.jiderhamn.classloader.leak.prevention.preinit;

import org.junit.Assume;

import se.jiderhamn.classloader.leak.prevention.support.JavaVersion;

/**
* Test cases for {@link SunGCInitiator}
* @author Mattias Jiderhamn
*/
public class SunGCInitiatorTest extends PreClassLoaderInitiatorTestBase<SunGCInitiator> {

@Override
public void firstShouldLeak() throws Exception {
// Leak does not occur any more with JDK11+ (and maybe 9 and 10 - not tested)
Assume.assumeTrue(JavaVersion.IS_JAVA_10_OR_EARLIER);
super.firstShouldLeak();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package se.jiderhamn.classloader.leak.prevention.support;

/**
* A few helper functions to use as a condition for JDK-dependant unit tests.
*/
public class JavaVersion {

public static final String JAVA_SPECIFICATION_VERSION = System.getProperty("java.specification.version");

/**
* Is {@code true} if this is Java version 1.6 (also 1.6.x versions).
*/
public static final boolean IS_JAVA_1_6 = isJavaVersionMatch("1.6");

/**
* Is {@code true} if this is Java version 1.7 (also 1.7.x versions).
*/
public static final boolean IS_JAVA_1_7 = isJavaVersionMatch("1.7");

/**
* Is {@code true} if this is Java version 1.8 (also 1.8.x versions).
*/
public static final boolean IS_JAVA_1_8 = isJavaVersionMatch("1.8");

/**
* Is {@code true} if this is Java version 1.8 or earlier (includes 1.8.x versions).
*/
public static final boolean IS_JAVA_1_8_OR_EARLIER = JavaVersion.IS_JAVA_1_6 || JavaVersion.IS_JAVA_1_7 || JavaVersion.IS_JAVA_1_8;

/**
* Is {@code true} if this is Java version 9 (also 9.x versions).
*/
public static final boolean IS_JAVA_9 = isJavaVersionMatch("9");

/**
* Is {@code true} if this is Java version 10 (also 10.x versions).
*/
public static final boolean IS_JAVA_10 = isJavaVersionMatch("10");

/**
* Is {@code true} if this is Java version 10 or earlier (includes 10.x versions).
*/
public static final boolean IS_JAVA_10_OR_EARLIER = IS_JAVA_1_8_OR_EARLIER || JavaVersion.IS_JAVA_9 || JavaVersion.IS_JAVA_10;

/**
* Is {@code true} if this is Java version 11 (also 11.x versions).
*/
public static final boolean IS_JAVA_11 = isJavaVersionMatch("11");

/**
* Is {@code true} if this is Java version 12 (also 12.x versions).
*/
public static final boolean IS_JAVA_12 = isJavaVersionMatch("12");

/**
* Is {@code true} if this is Java version 13 (also 13.x versions).
*/
public static final boolean IS_JAVA_13 = isJavaVersionMatch("13");

/**
* Is {@code true} if this is Java version 14 (also 14.x versions).
*/
public static final boolean IS_JAVA_14 = isJavaVersionMatch("14");

/**
* Is {@code true} if this is Java version 15 (also 15.x versions).
*/
public static final boolean IS_JAVA_15 = isJavaVersionMatch("15");

/**
* Is {@code true} if this is Java version 16 (also 16.x versions).
*/
public static final boolean IS_JAVA_16 = isJavaVersionMatch("16");

/**
* Is {@code true} if this is Java version 16 or earlier (includes 16.x versions).
*/
public static final boolean IS_JAVA_16_OR_EARLIER =
IS_JAVA_10_OR_EARLIER
|| JavaVersion.IS_JAVA_11
|| JavaVersion.IS_JAVA_12
|| JavaVersion.IS_JAVA_13
|| JavaVersion.IS_JAVA_14
|| JavaVersion.IS_JAVA_15
|| JavaVersion.IS_JAVA_16;

/**
* Is {@code true} if this is Java version 17 (also 17.x versions).
*/
public static final boolean IS_JAVA_17 = isJavaVersionMatch("17");

static boolean isJavaVersionMatch(final String versionPrefix) {
String version = JAVA_SPECIFICATION_VERSION;
if (version == null) {
return false;
}
return version.startsWith(versionPrefix);
}
}
39 changes: 37 additions & 2 deletions classloader-leak-prevention/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@
<artifactId>maven-compiler-plugin</artifactId>
<version>3.6.1</version>
<configuration>
<source>1.6</source>
<target>1.6</target>
<source>1.7</source>
<target>1.7</target>
<fork>true</fork>
<!-- Allow compiling against com.sun.xml.internal classes -->
<testCompilerArgument>-XDignore.symbol.file</testCompilerArgument>
Expand All @@ -125,6 +125,41 @@
</build>

<profiles>
<profile>
<id>surefire-java9orhigher</id>
<activation>
<jdk>(9,)</jdk>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>2.22.2</version>
<configuration>
<argLine>
--add-opens java.base/java.lang=ALL-UNNAMED
--add-opens java.base/java.net=ALL-UNNAMED
--add-opens java.base/java.util=ALL-UNNAMED
--add-opens java.base/java.security=ALL-UNNAMED
--add-opens java.base/javax.crypto=ALL-UNNAMED
--add-opens java.base/sun.reflect.misc=ALL-UNNAMED
--add-opens java.base/sun.security.action=ALL-UNNAMED
--add-opens java.desktop/java.beans=ALL-UNNAMED
--add-opens java.desktop/javax.imageio.spi=ALL-UNNAMED
--add-opens java.desktop/sun.java2d.opengl=ALL-UNNAMED
--add-opens java.desktop/sun.awt=ALL-UNNAMED
--add-opens java.desktop/sun.swing=ALL-UNNAMED
--add-opens java.desktop/sun.lwawt.macosx=ALL-UNNAMED
--add-opens java.rmi/sun.rmi.transport=ALL-UNNAMED
--add-opens java.management/sun.management=ALL-UNNAMED
--add-opens java.management/com.sun.jmx.interceptor=ALL-UNNAMED
</argLine>
</configuration>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>release</id>
<build>
Expand Down
Loading