use ScheduledExecutorService instanceof timer, fix timer cancelled problem. #1184

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,15 +50,15 @@ 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() {
+                try {
                     if (adviceListenerMap != null) {
                         for (Entry<ClassLoader, ClassLoaderAdviceListenerManager> entry : adviceListenerMap.entrySet()) {
                             ClassLoaderAdviceListenerManager adviceListenerManager = entry.getValue();
@@ -83,9 +84,15 @@ public class AdviceListenerManager {
                             }
                         }
                     }
+                } catch (Throwable e) {
+                    try {
+                        logger.error("clean AdviceListener error", e);
+                    } catch (Throwable t) {
+                        // ignore
                     }
-
-        }, 3000, 3000);
+                }
+            }
+        }, 3, 3, TimeUnit.SECONDS);
     }
 
     static private ConcurrentWeakKeyHashMap<ClassLoader, ClassLoaderAdviceListenerManager> adviceListenerMap = new ConcurrentWeakKeyHashMap<ClassLoader, ClassLoaderAdviceListenerManager>();

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;
     }

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<Integer, TimerTask> jobTimeoutTaskMap = new HashMap<Integer, TimerTask>();
+    private Map<Integer, JobTimeoutTask> jobTimeoutTaskMap = new HashMap<Integer, JobTimeoutTask>();
     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();
         }
     }
 }