From 4aac69fb4635cebf5fa4e12d5201ab6826006e1c Mon Sep 17 00:00:00 2001 From: hengyunabc Date: Sat, 6 Jun 2026 20:49:24 +0800 Subject: [PATCH] fix advice listener snapshot registration (#3209) --- .../core/advisor/AdviceListenerManager.java | 33 +++--- .../advisor/AdviceListenerManagerTest.java | 105 ++++++++++++++++++ 2 files changed, 120 insertions(+), 18 deletions(-) create mode 100644 core/src/test/java/com/taobao/arthas/core/advisor/AdviceListenerManagerTest.java diff --git a/core/src/main/java/com/taobao/arthas/core/advisor/AdviceListenerManager.java b/core/src/main/java/com/taobao/arthas/core/advisor/AdviceListenerManager.java index f2cdd414b..a88818dff 100644 --- a/core/src/main/java/com/taobao/arthas/core/advisor/AdviceListenerManager.java +++ b/core/src/main/java/com/taobao/arthas/core/advisor/AdviceListenerManager.java @@ -116,15 +116,7 @@ public class AdviceListenerManager { synchronized (this) { className = className.replace('/', '.'); String key = key(className, methodName, methodDesc); - - List listeners = map.get(key); - if (listeners == null) { - listeners = new ArrayList(); - map.put(key, listeners); - } - if (!listeners.contains(listener)) { - listeners.add(listener); - } + registerListener(key, listener); } } @@ -142,18 +134,23 @@ public class AdviceListenerManager { synchronized (this) { className = className.replace('/', '.'); String key = keyForTrace(className, owner, methodName, methodDesc); - - List listeners = map.get(key); - if (listeners == null) { - listeners = new ArrayList(); - map.put(key, listeners); - } - if (!listeners.contains(listener)) { - listeners.add(listener); - } + registerListener(key, listener); } } + private void registerListener(String key, AdviceListener listener) { + List listeners = map.get(key); + if (listeners != null && listeners.contains(listener)) { + return; + } + + List newListeners = listeners == null + ? new ArrayList() + : new ArrayList(listeners); + newListeners.add(listener); + map.put(key, newListeners); + } + public List queryTraceAdviceListeners(String className, String owner, String methodName, String methodDesc) { className = className.replace('/', '.'); diff --git a/core/src/test/java/com/taobao/arthas/core/advisor/AdviceListenerManagerTest.java b/core/src/test/java/com/taobao/arthas/core/advisor/AdviceListenerManagerTest.java new file mode 100644 index 000000000..4dab8e4e3 --- /dev/null +++ b/core/src/test/java/com/taobao/arthas/core/advisor/AdviceListenerManagerTest.java @@ -0,0 +1,105 @@ +package com.taobao.arthas.core.advisor; + +import java.util.Iterator; +import java.util.List; + +import org.assertj.core.api.Assertions; +import org.junit.Test; + +public class AdviceListenerManagerTest { + + @Test + public void queryAdviceListenersShouldReturnStableSnapshotWhenRegisteringAnotherListener() { + AdviceListenerManager.ClassLoaderAdviceListenerManager manager = + new AdviceListenerManager.ClassLoaderAdviceListenerManager(); + TestAdviceListener first = new TestAdviceListener(1); + TestAdviceListener second = new TestAdviceListener(2); + + manager.registerAdviceListener("demo.MathGame", "primeFactors", "(I)Ljava/util/List;", first); + + List listeners = manager.queryAdviceListeners("demo.MathGame", "primeFactors", + "(I)Ljava/util/List;"); + Iterator iterator = listeners.iterator(); + Assertions.assertThat(iterator.next()).isSameAs(first); + + manager.registerAdviceListener("demo.MathGame", "primeFactors", "(I)Ljava/util/List;", second); + + Assertions.assertThatCode(new org.assertj.core.api.ThrowableAssert.ThrowingCallable() { + @Override + public void call() { + while (iterator.hasNext()) { + iterator.next(); + } + } + }).doesNotThrowAnyException(); + Assertions.assertThat(listeners).containsExactly(first); + Assertions.assertThat(manager.queryAdviceListeners("demo.MathGame", "primeFactors", "(I)Ljava/util/List;")) + .containsExactly(first, second); + } + + @Test + public void queryTraceAdviceListenersShouldReturnStableSnapshotWhenRegisteringAnotherListener() { + AdviceListenerManager.ClassLoaderAdviceListenerManager manager = + new AdviceListenerManager.ClassLoaderAdviceListenerManager(); + TestAdviceListener first = new TestAdviceListener(1); + TestAdviceListener second = new TestAdviceListener(2); + + manager.registerTraceAdviceListener("demo.MathGame", "java/lang/StringBuilder", "append", + "(I)Ljava/lang/StringBuilder;", first); + + List listeners = manager.queryTraceAdviceListeners("demo.MathGame", "java/lang/StringBuilder", + "append", "(I)Ljava/lang/StringBuilder;"); + Iterator iterator = listeners.iterator(); + Assertions.assertThat(iterator.next()).isSameAs(first); + + manager.registerTraceAdviceListener("demo.MathGame", "java/lang/StringBuilder", "append", + "(I)Ljava/lang/StringBuilder;", second); + + Assertions.assertThatCode(new org.assertj.core.api.ThrowableAssert.ThrowingCallable() { + @Override + public void call() { + while (iterator.hasNext()) { + iterator.next(); + } + } + }).doesNotThrowAnyException(); + Assertions.assertThat(listeners).containsExactly(first); + Assertions.assertThat(manager.queryTraceAdviceListeners("demo.MathGame", "java/lang/StringBuilder", "append", + "(I)Ljava/lang/StringBuilder;")).containsExactly(first, second); + } + + private static class TestAdviceListener implements AdviceListener { + private final long id; + + private TestAdviceListener(long id) { + this.id = id; + } + + @Override + public long id() { + return id; + } + + @Override + public void create() { + } + + @Override + public void destroy() { + } + + @Override + public void before(Class clazz, String methodName, String methodDesc, Object target, Object[] args) { + } + + @Override + public void afterReturning(Class clazz, String methodName, String methodDesc, Object target, Object[] args, + Object returnObject) { + } + + @Override + public void afterThrowing(Class clazz, String methodName, String methodDesc, Object target, Object[] args, + Throwable throwable) { + } + } +}