From 6322374d81d9e1374c65f5594cf784c37a459fe9 Mon Sep 17 00:00:00 2001 From: hengyunabc Date: Sun, 24 May 2020 00:19:38 +0800 Subject: [PATCH] use ScheduledExecutorService instanceof timer, fix timer cancelled problem. #1184 --- .../core/advisor/AdviceListenerManager.java | 57 +++++++++++-------- .../arthas/core/server/ArthasBootstrap.java | 10 +++- .../system/impl/GlobalJobControllerImpl.java | 23 ++++---- 3 files changed, 49 insertions(+), 41 deletions(-) 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 bb3aca3d7..1cf3fb2e7 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 @@ -3,10 +3,11 @@ package com.taobao.arthas.core.advisor; import java.util.ArrayList; import java.util.List; import java.util.Map.Entry; -import java.util.Timer; -import java.util.TimerTask; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; +import com.alibaba.arthas.deps.org.slf4j.Logger; +import com.alibaba.arthas.deps.org.slf4j.LoggerFactory; import com.taobao.arthas.common.concurrent.ConcurrentWeakKeyHashMap; import com.taobao.arthas.core.server.ArthasBootstrap; import com.taobao.arthas.core.shell.system.ExecStatus; @@ -49,43 +50,49 @@ import com.taobao.arthas.core.shell.system.ProcessAware; * */ public class AdviceListenerManager { - - private static Timer timer = ArthasBootstrap.getInstance().getTimer(); + private static final Logger logger = LoggerFactory.getLogger(AdviceListenerManager.class); private static final FakeBootstrapClassLoader FAKEBOOTSTRAPCLASSLOADER = new FakeBootstrapClassLoader(); static { - timer.scheduleAtFixedRate(new TimerTask() { - + // 清理失效的 AdviceListener + ArthasBootstrap.getInstance().getScheduledExecutorService().scheduleWithFixedDelay(new Runnable() { @Override public void run() { - if (adviceListenerMap != null) { - for (Entry entry : adviceListenerMap.entrySet()) { - ClassLoaderAdviceListenerManager adviceListenerManager = entry.getValue(); - synchronized (adviceListenerManager) { - for (Entry> eee : adviceListenerManager.map.entrySet()) { - List listeners = eee.getValue(); - List newResult = new ArrayList(); - for (AdviceListener listener : listeners) { - if (listener instanceof ProcessAware) { - ProcessAware processAware = (ProcessAware) listener; - ExecStatus status = processAware.getProcess().status(); - if (!status.equals(ExecStatus.TERMINATED)) { - newResult.add(listener); + try { + if (adviceListenerMap != null) { + for (Entry entry : adviceListenerMap.entrySet()) { + ClassLoaderAdviceListenerManager adviceListenerManager = entry.getValue(); + synchronized (adviceListenerManager) { + for (Entry> eee : adviceListenerManager.map.entrySet()) { + List listeners = eee.getValue(); + List newResult = new ArrayList(); + for (AdviceListener listener : listeners) { + if (listener instanceof ProcessAware) { + ProcessAware processAware = (ProcessAware) listener; + ExecStatus status = processAware.getProcess().status(); + if (!status.equals(ExecStatus.TERMINATED)) { + newResult.add(listener); + } } } - } - if (newResult.size() != listeners.size()) { - adviceListenerManager.map.put(eee.getKey(), newResult); - } + if (newResult.size() != listeners.size()) { + adviceListenerManager.map.put(eee.getKey(), newResult); + } + } } } } + } catch (Throwable e) { + try { + logger.error("clean AdviceListener error", e); + } catch (Throwable t) { + // ignore + } } } - - }, 3000, 3000); + }, 3, 3, TimeUnit.SECONDS); } static private ConcurrentWeakKeyHashMap adviceListenerMap = new ConcurrentWeakKeyHashMap(); diff --git a/core/src/main/java/com/taobao/arthas/core/server/ArthasBootstrap.java b/core/src/main/java/com/taobao/arthas/core/server/ArthasBootstrap.java index 1b898a98c..e30bf900b 100644 --- a/core/src/main/java/com/taobao/arthas/core/server/ArthasBootstrap.java +++ b/core/src/main/java/com/taobao/arthas/core/server/ArthasBootstrap.java @@ -14,8 +14,8 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Properties; import java.util.Timer; -import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -70,7 +70,7 @@ public class ArthasBootstrap { private Instrumentation instrumentation; private Thread shutdown; private ShellServer shellServer; - private ExecutorService executorService; + private ScheduledExecutorService executorService; private TunnelClient tunnelClient; private File arthasOutputDir; @@ -98,7 +98,7 @@ public class ArthasBootstrap { // 4. start agent server bind(configure); - executorService = Executors.newCachedThreadPool(new ThreadFactory() { + executorService = Executors.newScheduledThreadPool(1, new ThreadFactory() { @Override public Thread newThread(Runnable r) { final Thread t = new Thread(r, "as-command-execute-daemon"); @@ -389,6 +389,10 @@ public class ArthasBootstrap { return this.timer; } + public ScheduledExecutorService getScheduledExecutorService() { + return this.executorService; + } + public Instrumentation getInstrumentation() { return this.instrumentation; } diff --git a/core/src/main/java/com/taobao/arthas/core/shell/system/impl/GlobalJobControllerImpl.java b/core/src/main/java/com/taobao/arthas/core/shell/system/impl/GlobalJobControllerImpl.java index 0f17507ea..f0900800a 100644 --- a/core/src/main/java/com/taobao/arthas/core/shell/system/impl/GlobalJobControllerImpl.java +++ b/core/src/main/java/com/taobao/arthas/core/shell/system/impl/GlobalJobControllerImpl.java @@ -4,7 +4,6 @@ import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.TimerTask; import java.util.concurrent.TimeUnit; import com.alibaba.arthas.deps.org.slf4j.Logger; @@ -22,7 +21,7 @@ import com.taobao.arthas.core.shell.system.Job; * @author gehui 2017年7月31日 上午11:55:41 */ public class GlobalJobControllerImpl extends JobControllerImpl { - private Map jobTimeoutTaskMap = new HashMap(); + private Map jobTimeoutTaskMap = new HashMap(); private static final Logger logger = LoggerFactory.getLogger(GlobalJobControllerImpl.class); @Override @@ -42,7 +41,7 @@ public class GlobalJobControllerImpl extends JobControllerImpl { @Override public boolean removeJob(int id) { - TimerTask jobTimeoutTask = jobTimeoutTaskMap.remove(id); + JobTimeoutTask jobTimeoutTask = jobTimeoutTaskMap.remove(id); if (jobTimeoutTask != null) { jobTimeoutTask.cancel(); } @@ -56,9 +55,10 @@ public class GlobalJobControllerImpl extends JobControllerImpl { /* * 达到超时时间将会停止job */ - TimerTask jobTimeoutTask = new JobTimeoutTask(job); - Date timeoutDate = new Date(System.currentTimeMillis() + (getJobTimeoutInSecond() * 1000)); - ArthasBootstrap.getInstance().getTimer().schedule(jobTimeoutTask, timeoutDate); + JobTimeoutTask jobTimeoutTask = new JobTimeoutTask(job); + long jobTimeoutInSecond = getJobTimeoutInSecond(); + Date timeoutDate = new Date(System.currentTimeMillis() + (jobTimeoutInSecond * 1000)); + ArthasBootstrap.getInstance().getScheduledExecutorService().schedule(jobTimeoutTask, jobTimeoutInSecond, TimeUnit.SECONDS); jobTimeoutTaskMap.put(job.id(), jobTimeoutTask); job.setTimeoutDate(timeoutDate); @@ -99,8 +99,8 @@ public class GlobalJobControllerImpl extends JobControllerImpl { return result; } - private static class JobTimeoutTask extends TimerTask { - Job job; + private static class JobTimeoutTask implements Runnable { + private Job job; public JobTimeoutTask(Job job) { this.job = job; @@ -110,15 +110,12 @@ public class GlobalJobControllerImpl extends JobControllerImpl { public void run() { if (job != null) { job.terminate(); + job = null; } } - @Override - public boolean cancel() { - // clear job reference from timer - // fix issue: https://github.com/alibaba/arthas/issues/1189 + public void cancel() { job = null; - return super.cancel(); } } }