Leak was introduced in commit 3c80a36527, fixing #128, but introducing #288 instead, which was the lesser of two evils, but still bad for some users unwilling to use native AspectJ syntax for their aspects, avoiding the problem. Relates to #288. Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>tags/V1_9_21_2
@@ -0,0 +1,12 @@ | |||
import org.aspectj.lang.ProceedingJoinPoint; | |||
import org.aspectj.lang.annotation.Around; | |||
import org.aspectj.lang.annotation.Aspect; | |||
@Aspect | |||
public class FirstAspect { | |||
@Around("execution(* doSomething())") | |||
public Object doAround(ProceedingJoinPoint joinPoint) throws Throwable { | |||
System.out.println(getClass().getSimpleName()); | |||
return joinPoint.proceed(); | |||
} | |||
} |
@@ -0,0 +1,18 @@ | |||
import java.util.concurrent.ExecutionException; | |||
import java.util.concurrent.ExecutorService; | |||
import java.util.concurrent.Future; | |||
public class MemoryHog { | |||
final ExecutorService taskManager; | |||
// Use 128 MB of data, then run with -Xmx1G for 10 threads or -Xmx512M for 5 threads | |||
final byte[] someBigData = new byte[1024 * 1024 * 128]; | |||
public MemoryHog(final ExecutorService executorService) { | |||
taskManager = executorService; | |||
} | |||
public void doSomething() throws ExecutionException, InterruptedException { | |||
Future<?> future = taskManager.submit(() -> System.out.println("Executing task")); | |||
future.get(); | |||
} | |||
} |
@@ -0,0 +1,104 @@ | |||
import java.lang.reflect.Field; | |||
import java.util.ArrayList; | |||
import java.util.List; | |||
import java.util.Set; | |||
import java.util.concurrent.ExecutorService; | |||
import java.util.concurrent.Executors; | |||
public class NestedAroundClosureMemoryLeakTest { | |||
public static void main(String[] args) throws Exception { | |||
if (args.length > 0 && "oom".equals(args[0])) | |||
testNoMemoryLeak_SystemShouldNotRunOutOfMemory(); | |||
else | |||
testNoMemoryLeak_InheritableThreadLocalCleared(); | |||
} | |||
/** | |||
* Tests that the inheritable thread-locals of the spawned threads are either null or contain all null elements | |||
*/ | |||
public static void testNoMemoryLeak_InheritableThreadLocalCleared() throws Exception { | |||
int numThreadPools = 1; | |||
List<ExecutorService> executorServices = createExecutorServicesWithFixedThreadPools(numThreadPools); | |||
try { | |||
executeTasksAndGC(executorServices); | |||
Field mapField = Thread.class.getDeclaredField("inheritableThreadLocals"); | |||
mapField.setAccessible(true); | |||
Set<Thread> threads = Thread.getAllStackTraces().keySet(); | |||
threads.stream() | |||
.filter(thread -> thread.getName().contains("pool")) | |||
.forEach(thread -> { | |||
try { | |||
Object inheritableThreadLocals = mapField.get(thread); | |||
if (inheritableThreadLocals != null) { | |||
Field tableField = inheritableThreadLocals.getClass().getDeclaredField("table"); | |||
tableField.setAccessible(true); | |||
Object[] inheritableThreadLocalTable = (Object[]) tableField.get(inheritableThreadLocals); | |||
if (inheritableThreadLocalTable != null) { | |||
for (Object entry : inheritableThreadLocalTable) | |||
assert entry == null : "All inheritable thread-locals should be null after GC"; | |||
} | |||
} | |||
} | |||
catch (Exception e) { | |||
throw new RuntimeException(e); | |||
} | |||
}); | |||
System.out.println("Test passed - all inheritable thread-locals are null after GC"); | |||
} | |||
finally { | |||
for (ExecutorService executorService : executorServices) | |||
executorService.shutdown(); | |||
} | |||
} | |||
/** | |||
* Executes tasks in multiple threads, using one executor service with a fixed thread pool of size 1 per task. This | |||
* ensures that each spawned thread gets initialised for the first time and allocates memory for inheritable | |||
* thread-locals, exposing possible memory leaks when running @AspectJ aspects with non-inlined, nested around advices | |||
* in multi-thread situations. | |||
* <p> | |||
* If each thread allocates memory for a stack of around closures (memory leak case) according to | |||
* <a href="https://github.com/eclipse-aspectj/aspectj/issues/288">GitHub issue 288</a>, the program will run out of | |||
* memory. When fixed, this should no longer happen. | |||
* <p> | |||
* Run this test e.g. with {@code -Xmx1G} for 10 x 128 MB memory consumption to ensure an out of memory error in the | |||
* leak case. Any other appropriate combination of number of threads and memory limit is also OK. | |||
*/ | |||
public static void testNoMemoryLeak_SystemShouldNotRunOutOfMemory() throws Exception { | |||
int numThreadPools = 5; | |||
List<ExecutorService> executorServices = createExecutorServicesWithFixedThreadPools(numThreadPools); | |||
try { | |||
executeTasksAndGC(executorServices); | |||
System.out.println("Test passed - no OutOfMemoryError due to inheritable thread-locals memory leak"); | |||
} | |||
finally { | |||
for (ExecutorService executorService : executorServices) | |||
executorService.shutdown(); | |||
} | |||
} | |||
private static List<ExecutorService> createExecutorServicesWithFixedThreadPools(int count) { | |||
List<ExecutorService> executorServiceList = new ArrayList<>(count); | |||
for (int i = 0; i < count; i++) | |||
executorServiceList.add(Executors.newFixedThreadPool(1)); | |||
return executorServiceList; | |||
} | |||
private static void executeTasksAndGC(List<ExecutorService> executorServices) throws Exception { | |||
for (ExecutorService executorService : executorServices) | |||
new MemoryHog(executorService).doSomething(); | |||
System.out.println("Finished executing tasks"); | |||
// Best effort GC | |||
System.gc(); | |||
System.out.println("Finished executing GC"); | |||
// Sleep to take a memory dump | |||
// Thread.sleep(500000); | |||
} | |||
} |
@@ -0,0 +1,12 @@ | |||
import org.aspectj.lang.ProceedingJoinPoint; | |||
import org.aspectj.lang.annotation.Around; | |||
import org.aspectj.lang.annotation.Aspect; | |||
@Aspect | |||
public class SecondAspect { | |||
@Around("execution(* doSomething())") | |||
public Object doAround(ProceedingJoinPoint joinPoint) throws Throwable { | |||
System.out.println(getClass().getSimpleName()); | |||
return joinPoint.proceed(); | |||
} | |||
} |
@@ -23,6 +23,14 @@ public class Bugs1921Tests extends XMLBasedAjcTestCase { | |||
runTest("shared cache negative test"); | |||
} | |||
public void testGitHub_288_AssertionError() { | |||
runTest("memory leak for @AspectJ nested, non-inlined around-advice - AssertionError"); | |||
} | |||
public void testGitHub_288_OutOfMemoryError() { | |||
runTest("memory leak for @AspectJ nested, non-inlined around-advice - OutOfMemoryError"); | |||
} | |||
public static Test suite() { | |||
return XMLBasedAjcTestCase.loadSuite(Bugs1921Tests.class); | |||
} |
@@ -393,4 +393,56 @@ | |||
</run> | |||
</ajc-test> | |||
<!-- | |||
https://github.com/eclipse-aspectj/aspectj/issues/288, | |||
https://github.com/eclipse-aspectj/aspectj/issues/141, AspectJ 1.9.21.2 | |||
--> | |||
<ajc-test dir="bugs1921/github_288" title="memory leak for @AspectJ nested, non-inlined around-advice - AssertionError"> | |||
<compile files="NestedAroundClosureMemoryLeakTest.java MemoryHog.java FirstAspect.aj SecondAspect.aj" options="-1.8 -XnoInline"/> | |||
<run class="NestedAroundClosureMemoryLeakTest" vmargs="-ea --add-opens java.base/java.lang=ALL-UNNAMED"> | |||
<stdout> | |||
<line text="FirstAspect"/> | |||
<line text="SecondAspect"/> | |||
<line text="Executing task"/> | |||
<line text="Finished executing tasks"/> | |||
<line text="Finished executing GC"/> | |||
<line text="Test passed - all inheritable thread-locals are null after GC"/> | |||
</stdout> | |||
<!-- No AssertionError on stderr--> | |||
<stderr/> | |||
</run> | |||
</ajc-test> | |||
<!-- | |||
https://github.com/eclipse-aspectj/aspectj/issues/288, | |||
https://github.com/eclipse-aspectj/aspectj/issues/141, AspectJ 1.9.21.2 | |||
--> | |||
<ajc-test dir="bugs1921/github_288" title="memory leak for @AspectJ nested, non-inlined around-advice - OutOfMemoryError"> | |||
<compile files="NestedAroundClosureMemoryLeakTest.java MemoryHog.java FirstAspect.aj SecondAspect.aj" options="-1.8 -XnoInline"/> | |||
<run class="NestedAroundClosureMemoryLeakTest" vmargs="-ea --add-opens java.base/java.lang=ALL-UNNAMED -Xmx512M" options="oom"> | |||
<stdout> | |||
<line text="FirstAspect"/> | |||
<line text="SecondAspect"/> | |||
<line text="Executing task"/> | |||
<line text="FirstAspect"/> | |||
<line text="SecondAspect"/> | |||
<line text="Executing task"/> | |||
<line text="FirstAspect"/> | |||
<line text="SecondAspect"/> | |||
<line text="Executing task"/> | |||
<line text="FirstAspect"/> | |||
<line text="SecondAspect"/> | |||
<line text="Executing task"/> | |||
<line text="FirstAspect"/> | |||
<line text="SecondAspect"/> | |||
<line text="Executing task"/> | |||
<line text="Finished executing tasks"/> | |||
<line text="Finished executing GC"/> | |||
<line text="Test passed - no OutOfMemoryError due to inheritable thread-locals memory leak"/> | |||
</stdout> | |||
<!-- No fatal OutOfMemoryError on stderr --> | |||
<stderr/> | |||
</run> | |||
</ajc-test> | |||
</suite> |