From a969bd35f933f185217aadc40644bdeea13aa781 Mon Sep 17 00:00:00 2001 From: mpet Date: Tue, 5 Sep 2023 12:02:30 +0200 Subject: [PATCH] JENKINS-71798: TimeoutService threads are left after closing connection (#155) * JENKINS-71798: TimeoutService threads are left after closing connection * Update src/com/trilead/ssh2/util/TimeoutService.java Co-authored-by: Ivan Fernandez Calvo * Update TimeoutService.java --------- Co-authored-by: Ivan Fernandez Calvo --- src/com/trilead/ssh2/Connection.java | 7 ++-- src/com/trilead/ssh2/util/TimeoutService.java | 35 ++++++++++++------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/com/trilead/ssh2/Connection.java b/src/com/trilead/ssh2/Connection.java index e43bde16..3149cb8d 100644 --- a/src/com/trilead/ssh2/Connection.java +++ b/src/com/trilead/ssh2/Connection.java @@ -665,7 +665,8 @@ final class TimeoutState if (kexTimeout < 0) throw new IllegalArgumentException("kexTimeout must be non-negative!"); - final TimeoutState state = new TimeoutState(); + final TimeoutState state = new TimeoutState(); + final TimeoutService timeoutService = new TimeoutService(hostname); tm = new TransportManager(hostname, port, sourceAddress); @@ -713,7 +714,7 @@ public void run() long timeoutHorizont = System.currentTimeMillis() + kexTimeout; - token = TimeoutService.addTimeoutHandler(timeoutHorizont, timeoutHandler); + token = timeoutService.addTimeoutHandler(timeoutHorizont, timeoutHandler); } try @@ -737,7 +738,7 @@ public void run() if (token != null) { - TimeoutService.cancelTimeoutHandler(token); + timeoutService.cancelTimeoutHandler(token); /* Were we too late? */ diff --git a/src/com/trilead/ssh2/util/TimeoutService.java b/src/com/trilead/ssh2/util/TimeoutService.java index 13a047ff..7ff784ea 100644 --- a/src/com/trilead/ssh2/util/TimeoutService.java +++ b/src/com/trilead/ssh2/util/TimeoutService.java @@ -3,6 +3,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -20,25 +21,33 @@ * @version $Id: TimeoutService.java,v 1.1 2007/10/15 12:49:57 cplattne Exp $ */ public class TimeoutService { - - private final static ThreadFactory threadFactory = new ThreadFactory() { - - private AtomicInteger count = new AtomicInteger(); - + + + private ScheduledFuture scheduledFuture; + private final String hostname; + private final ThreadFactory threadFactory = new ThreadFactory() { + private AtomicInteger count = new AtomicInteger(); + @Override public Thread newThread(Runnable r) { int threadNumber = count.incrementAndGet(); - String threadName = "TimeoutService-" + threadNumber; + String threadName = "Trilead_TimeoutService_" + hostname + "-" + threadNumber; Thread thread = new Thread(r, threadName); thread.setDaemon(true); return thread; } }; + private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(threadFactory); + + public TimeoutService(String hostname){ + this.hostname = hostname; + } + + - private final static ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(20, threadFactory); - public static class TimeoutToken implements Runnable { + public class TimeoutToken implements Runnable { private Runnable handler; - private boolean cancelled = false; + private volatile boolean cancelled = false; public void run() { if (!cancelled) { @@ -54,14 +63,14 @@ public void run() { * @param handler handler * @return a TimeoutToken that can be used to cancel the timeout. */ - public static final TimeoutToken addTimeoutHandler(long runTime, Runnable handler) { + public TimeoutToken addTimeoutHandler(long runTime, Runnable handler) { TimeoutToken token = new TimeoutToken(); token.handler = handler; long delay = runTime - System.currentTimeMillis(); if (delay < 0) { delay = 0; } - scheduler.schedule(token, delay, TimeUnit.MILLISECONDS); + scheduledFuture = scheduler.schedule(token, delay, TimeUnit.MILLISECONDS); return token; } @@ -70,7 +79,9 @@ public static final TimeoutToken addTimeoutHandler(long runTime, Runnable handle * * @param token token to be canceled. */ - public static final void cancelTimeoutHandler(TimeoutToken token) { + public void cancelTimeoutHandler(TimeoutToken token) { token.cancelled = true; + scheduledFuture.cancel(true); + scheduler.shutdownNow(); } }