Skip to content

Commit

Permalink
Resolve unit test failures.
Browse files Browse the repository at this point in the history
  • Loading branch information
@brettwooldridge
brettwooldridge committed Apr 19, 2017
1 parent 8db418f commit f1f7873
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 41 deletions.
2 changes: 1 addition & 1 deletion pom.xml
View file
Expand Up @@ -372,7 +372,7 @@
<argLine>${surefireArgLine} ${sureFireOptions9}</argLine> <argLine>${surefireArgLine} ${sureFireOptions9}</argLine>
<!-- Skips unit tests if the value of skip.unit.tests property is true --> <!-- Skips unit tests if the value of skip.unit.tests property is true -->
<skipTests>${skip.unit.tests}</skipTests> <skipTests>${skip.unit.tests}</skipTests>
<reuseForks>true</reuseForks> <reuseForks>false</reuseForks>
</configuration> </configuration>
</plugin> </plugin>


Expand Down
113 changes: 73 additions & 40 deletions src/test/java/com/zaxxer/hikari/util/TomcatConcurrentBagLeakTest.java
View file
Expand Up @@ -16,30 +16,29 @@


package com.zaxxer.hikari.util; package com.zaxxer.hikari.util;


import static com.zaxxer.hikari.pool.TestElf.isJava9; import com.zaxxer.hikari.util.ConcurrentBag.IConcurrentBagEntry;
import static java.util.concurrent.TimeUnit.MILLISECONDS; import org.junit.FixMethodOrder;
import static org.junit.Assert.assertNotNull; import org.junit.Test;
import static org.junit.Assert.assertNull; import org.junit.runners.MethodSorters;
import static org.junit.Assume.assumeTrue; import org.slf4j.Logger;
import org.slf4j.LoggerFactory;


import java.io.DataInputStream; import java.io.DataInputStream;
import java.io.IOException; import java.io.IOException;
import java.lang.ref.Reference; import java.lang.ref.Reference;
import java.lang.reflect.Field; import java.lang.reflect.Field;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.net.URL; import java.net.URL;
import java.util.Collection;
import java.util.ConcurrentModificationException;
import java.util.Iterator;
import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletableFuture;


import com.zaxxer.hikari.pool.TestElf; import static com.zaxxer.hikari.pool.TestElf.isJava9;
import org.junit.Assume; import static java.util.concurrent.TimeUnit.MILLISECONDS;
import org.junit.Before; import static org.junit.Assert.assertNotNull;
import org.junit.FixMethodOrder; import static org.junit.Assert.assertNull;
import org.junit.Test; import static org.junit.Assume.assumeTrue;
import org.junit.runners.MethodSorters;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.zaxxer.hikari.util.ConcurrentBag.IConcurrentBagEntry;


/** /**
* @author Brett Wooldridge * @author Brett Wooldridge
Expand Down Expand Up @@ -136,12 +135,15 @@ public int getState()
} }
} }


@SuppressWarnings("unused")
public static class FauxWebContext public static class FauxWebContext
{ {
private static final Logger log = LoggerFactory.getLogger(FauxWebContext.class); private static final Logger log = LoggerFactory.getLogger(FauxWebContext.class);


@SuppressWarnings("WeakerAccess")
public Exception failureException; public Exception failureException;


@SuppressWarnings("unused")
public void createConcurrentBag() throws InterruptedException public void createConcurrentBag() throws InterruptedException
{ {
try (ConcurrentBag<PoolEntry> bag = new ConcurrentBag<>((x) -> CompletableFuture.completedFuture(Boolean.TRUE))) { try (ConcurrentBag<PoolEntry> bag = new ConcurrentBag<>((x) -> CompletableFuture.completedFuture(Boolean.TRUE))) {
Expand Down Expand Up @@ -178,19 +180,19 @@ private void checkThreadLocalsForLeaks()
Method expungeStaleEntriesMethod = tlmClass.getDeclaredMethod("expungeStaleEntries"); Method expungeStaleEntriesMethod = tlmClass.getDeclaredMethod("expungeStaleEntries");
expungeStaleEntriesMethod.setAccessible(true); expungeStaleEntriesMethod.setAccessible(true);


for (int i = 0; i < threads.length; i++) { for (Thread thread : threads) {
Object threadLocalMap; Object threadLocalMap;
if (threads[i] != null) { if (thread != null) {


// Clear the first map // Clear the first map
threadLocalMap = threadLocalsField.get(threads[i]); threadLocalMap = threadLocalsField.get(thread);
if (null != threadLocalMap) { if (null != threadLocalMap) {
expungeStaleEntriesMethod.invoke(threadLocalMap); expungeStaleEntriesMethod.invoke(threadLocalMap);
checkThreadLocalMapForLeaks(threadLocalMap, tableField); checkThreadLocalMapForLeaks(threadLocalMap, tableField);
} }


// Clear the second map // Clear the second map
threadLocalMap = inheritableThreadLocalsField.get(threads[i]); threadLocalMap = inheritableThreadLocalsField.get(thread);
if (null != threadLocalMap) { if (null != threadLocalMap) {
expungeStaleEntriesMethod.invoke(threadLocalMap); expungeStaleEntriesMethod.invoke(threadLocalMap);
checkThreadLocalMapForLeaks(threadLocalMap, tableField); checkThreadLocalMapForLeaks(threadLocalMap, tableField);
Expand All @@ -199,7 +201,7 @@ private void checkThreadLocalsForLeaks()
} }
} }
catch (Throwable t) { catch (Throwable t) {
log.warn("Failed to check for ThreadLocal references for web application [{}]", t); log.warn("Failed to check for ThreadLocal references for web application [{}]", getContextName(), t);
failureException = new Exception(); failureException = new Exception();
} }
} }
Expand All @@ -221,8 +223,7 @@ private void checkThreadLocalMapForLeaks(Object map, Field internalTableField) t
if (map != null) { if (map != null) {
Object[] table = (Object[]) internalTableField.get(map); Object[] table = (Object[]) internalTableField.get(map);
if (table != null) { if (table != null) {
for (int j = 0; j < table.length; j++) { for (Object obj : table) {
Object obj = table[j];
if (obj != null) { if (obj != null) {
boolean keyLoadedByWebapp = false; boolean keyLoadedByWebapp = false;
boolean valueLoadedByWebapp = false; boolean valueLoadedByWebapp = false;
Expand All @@ -245,8 +246,7 @@ private void checkThreadLocalMapForLeaks(Object map, Field internalTableField) t
args[1] = getPrettyClassName(key.getClass()); args[1] = getPrettyClassName(key.getClass());
try { try {
args[2] = key.toString(); args[2] = key.toString();
} } catch (Exception e) {
catch (Exception e) {
log.warn("Unable to determine string representation of key of type [{}]", args[1], e); log.warn("Unable to determine string representation of key of type [{}]", args[1], e);
args[2] = "Unknown"; args[2] = "Unknown";
} }
Expand All @@ -255,30 +255,27 @@ private void checkThreadLocalMapForLeaks(Object map, Field internalTableField) t
args[3] = getPrettyClassName(value.getClass()); args[3] = getPrettyClassName(value.getClass());
try { try {
args[4] = value.toString(); args[4] = value.toString();
} } catch (Exception e) {
catch (Exception e) {
log.warn("webappClassLoader.checkThreadLocalsForLeaks.badValue {}", args[3], e); log.warn("webappClassLoader.checkThreadLocalsForLeaks.badValue {}", args[3], e);
args[4] = "Unknown"; args[4] = "Unknown";
} }
} }


if (valueLoadedByWebapp) { if (valueLoadedByWebapp) {
log.error("The web application [{}] created a ThreadLocal with key " + log.error("The web application [{}] created a ThreadLocal with key " +
"(value [{}]) and a value of type [{}] (value [{}]) but failed to remove " + "(value [{}]) and a value of type [{}] (value [{}]) but failed to remove " +
"it when the web application was stopped. Threads are going to be renewed " + "it when the web application was stopped. Threads are going to be renewed " +
"over time to try and avoid a probable memory leak.", args); "over time to try and avoid a probable memory leak.", args);
failureException = new Exception(); failureException = new Exception();
} } else if (value == null) {
else if (value == null) {
log.debug("The web application [{}] created a ThreadLocal with key of type [{}] " + log.debug("The web application [{}] created a ThreadLocal with key of type [{}] " +
"(value [{}]). The ThreadLocal has been correctly set to null and the " + "(value [{}]). The ThreadLocal has been correctly set to null and the " +
"key will be removed by GC.", args); "key will be removed by GC.", args);
failureException = new Exception(); failureException = new Exception();
} } else {
else {
log.debug("The web application [{}] created a ThreadLocal with key of type [{}] " + log.debug("The web application [{}] created a ThreadLocal with key of type [{}] " +
"(value [{}]) and a value of type [{}] (value [{}]). Since keys are only " + "(value [{}]) and a value of type [{}] (value [{}]). Since keys are only " +
"weakly held by the ThreadLocal Map this is not a memory leak.", args); "weakly held by the ThreadLocal Map this is not a memory leak.", args);
failureException = new Exception(); failureException = new Exception();
} }
} }
Expand All @@ -288,9 +285,45 @@ else if (value == null) {
} }
} }


private boolean loadedByThisOrChild(Object key) /**
{ * @param o object to test, may be null
return key.getClass().getClassLoader() == this.getClass().getClassLoader(); * @return <code>true</code> if o has been loaded by the current classloader
* or one of its descendants.
*/
private boolean loadedByThisOrChild(Object o) {
if (o == null) {
return false;
}

Class<?> clazz;
if (o instanceof Class) {
clazz = (Class<?>) o;
} else {
clazz = o.getClass();
}

ClassLoader cl = clazz.getClassLoader();
while (cl != null) {
if (cl == this.getClass().getClassLoader()) {
return true;
}
cl = cl.getParent();
}

if (o instanceof Collection<?>) {
Iterator<?> iter = ((Collection<?>) o).iterator();
try {
while (iter.hasNext()) {
Object entry = iter.next();
if (loadedByThisOrChild(entry)) {
return true;
}
}
} catch (ConcurrentModificationException e) {
log.warn("Failed to check for ThreadLocal references for web application [{}]", getContextName(), e);
}
}
return false;
} }


/* /*
Expand Down

0 comments on commit f1f7873

Please sign in to comment.