From 856db5d97f329041751418b2cc43d7574e26144d Mon Sep 17 00:00:00 2001 From: Alexander Kriegisch Date: Wed, 10 Apr 2024 11:03:13 +0200 Subject: [PATCH] Add IT reproducing JoinPointImpl thread-locals memory leak Relates to #302. Signed-off-by: Alexander Kriegisch --- tests/bugs1922/github_302/FirstAspect.aj | 12 +++ .../NestedAroundClosureMemoryLeakTest.java | 86 +++++++++++++++++++ tests/bugs1922/github_302/SecondAspect.aj | 12 +++ tests/bugs1922/github_302/Task.java | 20 +++++ .../systemtest/ajc1922/Bugs1922Tests.java | 4 +- .../aspectj/systemtest/ajc1922/ajc1922.xml | 14 +++ 6 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 tests/bugs1922/github_302/FirstAspect.aj create mode 100644 tests/bugs1922/github_302/NestedAroundClosureMemoryLeakTest.java create mode 100644 tests/bugs1922/github_302/SecondAspect.aj create mode 100644 tests/bugs1922/github_302/Task.java diff --git a/tests/bugs1922/github_302/FirstAspect.aj b/tests/bugs1922/github_302/FirstAspect.aj new file mode 100644 index 000000000..4d99110db --- /dev/null +++ b/tests/bugs1922/github_302/FirstAspect.aj @@ -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(* toIntercept())") + public Object doAround(ProceedingJoinPoint joinPoint) throws Throwable { + //System.out.println(getClass().getSimpleName()); + return joinPoint.proceed(); + } +} diff --git a/tests/bugs1922/github_302/NestedAroundClosureMemoryLeakTest.java b/tests/bugs1922/github_302/NestedAroundClosureMemoryLeakTest.java new file mode 100644 index 000000000..9b3b2f828 --- /dev/null +++ b/tests/bugs1922/github_302/NestedAroundClosureMemoryLeakTest.java @@ -0,0 +1,86 @@ +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 { + + private static final int NUM_THREAD_POOLS = 4; + private static final int THREAD_POOL_SIZE = 3; + + public static void main(String[] args) throws Exception { + testNoMemoryLeak_ThreadLocalCleared(); + } + + /** + * Tests that the thread-locals of the spawned threads are either null or contain all null elements + */ + public static void testNoMemoryLeak_ThreadLocalCleared() throws Exception { + List executorServices = createExecutorServicesWithFixedThreadPools(); + try { + executeTasks(executorServices); + + Field mapField = Thread.class.getDeclaredField("threadLocals"); + mapField.setAccessible(true); + Set threads = Thread.getAllStackTraces().keySet(); + System.out.println("Number of pool threads = " + threads.stream().filter(thread -> thread.getName().contains("pool")).count()); + + threads.stream() + .filter(thread -> thread.getName().contains("pool")) + .forEach(thread -> { + try { + Object threadLocals = mapField.get(thread); + if (threadLocals != null) { + Field tableField = threadLocals.getClass().getDeclaredField("table"); + tableField.setAccessible(true); + Object[] threadLocalTable = (Object[]) tableField.get(threadLocals); + if (threadLocalTable != null) { + for (Object entry : threadLocalTable) { + if (entry == null) + continue; + Field entryValueField = entry.getClass().getDeclaredField("value"); + entryValueField.setAccessible(true); + throw new RuntimeException( + "All thread-locals should be null, but found entry with value " + entryValueField.get(entry) + ); + } + } + } + } + catch (RuntimeException rte) { + throw rte; + } + catch (Exception e) { + throw new RuntimeException(e); + } + }); + + System.out.println("Test passed - all thread-locals are null"); + } + finally { + for (ExecutorService executorService : executorServices) + executorService.shutdown(); + } + } + + private static List createExecutorServicesWithFixedThreadPools() { + List executorServiceList = new ArrayList<>(NestedAroundClosureMemoryLeakTest.NUM_THREAD_POOLS); + for (int i = 0; i < NestedAroundClosureMemoryLeakTest.NUM_THREAD_POOLS; i++) + executorServiceList.add(Executors.newFixedThreadPool(THREAD_POOL_SIZE)); + return executorServiceList; + } + + private static void executeTasks(List executorServices) throws Exception { + for (ExecutorService executorService : executorServices) { + for (int i = 0; i < THREAD_POOL_SIZE * 2; i++) + new Task(executorService).doSomething(); + } + System.out.println("Finished executing tasks"); + + // Sleep to take a memory dump + // Thread.sleep(500000); + } + +} diff --git a/tests/bugs1922/github_302/SecondAspect.aj b/tests/bugs1922/github_302/SecondAspect.aj new file mode 100644 index 000000000..5f088b63a --- /dev/null +++ b/tests/bugs1922/github_302/SecondAspect.aj @@ -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(* toIntercept())") + public Object doAround(ProceedingJoinPoint joinPoint) throws Throwable { + //System.out.println(getClass().getSimpleName()); + return joinPoint.proceed(); + } +} diff --git a/tests/bugs1922/github_302/Task.java b/tests/bugs1922/github_302/Task.java new file mode 100644 index 000000000..0d1ea6402 --- /dev/null +++ b/tests/bugs1922/github_302/Task.java @@ -0,0 +1,20 @@ +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; + +public class Task { + final ExecutorService taskManager; + + public Task(final ExecutorService executorService) { + taskManager = executorService; + } + + public void doSomething() throws ExecutionException, InterruptedException { + Future future = taskManager.submit(Task::toIntercept); + future.get(); + } + + public static void toIntercept() { + //System.out.println("Executing task") + } +} diff --git a/tests/src/test/java/org/aspectj/systemtest/ajc1922/Bugs1922Tests.java b/tests/src/test/java/org/aspectj/systemtest/ajc1922/Bugs1922Tests.java index 06211a179..6e402fa11 100644 --- a/tests/src/test/java/org/aspectj/systemtest/ajc1922/Bugs1922Tests.java +++ b/tests/src/test/java/org/aspectj/systemtest/ajc1922/Bugs1922Tests.java @@ -15,8 +15,8 @@ import org.aspectj.testing.XMLBasedAjcTestCase; */ public class Bugs1922Tests extends XMLBasedAjcTestCase { - public void testDummy() { - //runTest("dummy"); + public void testGitHub_302() { + runTest("thread-local around closure index is removed after innermost proceed"); } public static Test suite() { diff --git a/tests/src/test/resources/org/aspectj/systemtest/ajc1922/ajc1922.xml b/tests/src/test/resources/org/aspectj/systemtest/ajc1922/ajc1922.xml index d436032e2..e10fd9975 100644 --- a/tests/src/test/resources/org/aspectj/systemtest/ajc1922/ajc1922.xml +++ b/tests/src/test/resources/org/aspectj/systemtest/ajc1922/ajc1922.xml @@ -196,4 +196,18 @@ + + + + + + + + + + + + + + -- 2.39.5