Hook-up comprensive command cleanup (destroy)

6 files changed, 83 insertions, 35 deletions

diff --git a/src/main/java/com/gitblit/transport/ssh/SshDaemon.java b/src/main/java/com/gitblit/transport/ssh/SshDaemon.java
index aeb6ce51..9628cb85 100644
--- a/src/main/java/com/gitblit/transport/ssh/SshDaemon.java
+++ b/src/main/java/com/gitblit/transport/ssh/SshDaemon.java
@@ -166,6 +166,7 @@ public class SshDaemon {
 			run.set(false);
 
 			try {
+				((SshCommandFactory) sshd.getCommandFactory()).stop();
 				sshd.stop();
 			} catch (InterruptedException e) {
 				log.error("SSH Daemon stop interrupted", e);
diff --git a/src/main/java/com/gitblit/transport/ssh/commands/BaseCommand.java b/src/main/java/com/gitblit/transport/ssh/commands/BaseCommand.java
index 7c71ffa7..4162a400 100644
--- a/src/main/java/com/gitblit/transport/ssh/commands/BaseCommand.java
+++ b/src/main/java/com/gitblit/transport/ssh/commands/BaseCommand.java
@@ -48,6 +48,14 @@ public abstract class BaseCommand implements Command, SessionAware {
 
 	private static final Logger log = LoggerFactory.getLogger(BaseCommand.class);
 
+	private static final int PRIVATE_STATUS = 1 << 30;
+
+	public final static int STATUS_CANCEL = PRIVATE_STATUS | 1;
+
+	public final static int STATUS_NOT_FOUND = PRIVATE_STATUS | 2;
+
+	public final static int STATUS_NOT_ADMIN = PRIVATE_STATUS | 3;
+
 	protected InputStream in;
 
 	protected OutputStream out;
@@ -86,6 +94,8 @@ public abstract class BaseCommand implements Command, SessionAware {
 
 	@Override
 	public void destroy() {
+		log.debug("destroying " + getClass().getName());
+		session = null;
 		ctx = null;
 	}
 
diff --git a/src/main/java/com/gitblit/transport/ssh/commands/DispatchCommand.java b/src/main/java/com/gitblit/transport/ssh/commands/DispatchCommand.java
index 779f0b00..4783c053 100644
--- a/src/main/java/com/gitblit/transport/ssh/commands/DispatchCommand.java
+++ b/src/main/java/com/gitblit/transport/ssh/commands/DispatchCommand.java
@@ -48,11 +48,28 @@ public abstract class DispatchCommand extends BaseCommand {
 	private List<String> args = new ArrayList<String>();
 
 	private final Set<Class<? extends BaseCommand>> commands;
+	private final Map<String, DispatchCommand> dispatchers;
+	private final List<BaseCommand> instantiated;
 	private Map<String, Class<? extends BaseCommand>> map;
-	private Map<String, BaseCommand> dispatchers;
 
 	protected DispatchCommand() {
 		commands = new HashSet<Class<? extends BaseCommand>>();
+		dispatchers = Maps.newHashMap();
+		instantiated = new ArrayList<BaseCommand>();
+	}
+
+	@Override
+	public void destroy() {
+		super.destroy();
+		commands.clear();
+		map = null;
+
+		for (BaseCommand command : instantiated) {
+			command.destroy();
+		}
+		for (DispatchCommand dispatcher : dispatchers.values()) {
+			dispatcher.destroy();
+		}
 	}
 
 	protected void registerDispatcher(UserModel user, Class<? extends DispatchCommand> cmd) {
@@ -60,9 +77,6 @@ public abstract class DispatchCommand extends BaseCommand {
 			throw new RuntimeException(MessageFormat.format("{0} must be annotated with {1}!", cmd.getName(),
 					CommandMetaData.class.getName()));
 		}
-		if (dispatchers == null) {
-			dispatchers = Maps.newHashMap();
-		}
 
 		CommandMetaData meta = cmd.getAnnotation(CommandMetaData.class);
 		if (meta.admin() && !user.canAdmin()) {
@@ -108,10 +122,9 @@ public abstract class DispatchCommand extends BaseCommand {
 				CommandMetaData meta = cmd.getAnnotation(CommandMetaData.class);
 				map.put(meta.name(), cmd);
 			}
-			if (dispatchers != null) {
-				for (Map.Entry<String, BaseCommand> entry : dispatchers.entrySet()) {
-					map.put(entry.getKey(), entry.getValue().getClass());
-				}
+
+			for (Map.Entry<String, DispatchCommand> entry : dispatchers.entrySet()) {
+				map.put(entry.getKey(), entry.getValue().getClass());
 			}
 		}
 		return map;
@@ -163,6 +176,7 @@ public abstract class DispatchCommand extends BaseCommand {
 		BaseCommand cmd = null;
 		try {
 			cmd = c.newInstance();
+			instantiated.add(cmd);
 		} catch (Exception e) {
 			throw new UnloggedFailure(1, MessageFormat.format("Failed to instantiate {0} command", commandName));
 		}
diff --git a/src/main/java/com/gitblit/transport/ssh/commands/SshCommandFactory.java b/src/main/java/com/gitblit/transport/ssh/commands/SshCommandFactory.java
index 3eefcae7..8f7144de 100644
--- a/src/main/java/com/gitblit/transport/ssh/commands/SshCommandFactory.java
+++ b/src/main/java/com/gitblit/transport/ssh/commands/SshCommandFactory.java
@@ -20,6 +20,8 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -39,6 +41,7 @@ import com.gitblit.transport.ssh.SshDaemonClient;
 import com.gitblit.utils.IdGenerator;
 import com.gitblit.utils.WorkQueue;
 import com.google.common.util.concurrent.Atomics;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
 
 /**
  *
@@ -50,6 +53,7 @@ public class SshCommandFactory implements CommandFactory {
 
 	private final IGitblit gitblit;
 	private final ScheduledExecutorService startExecutor;
+	private final ExecutorService destroyExecutor;
 
 	public SshCommandFactory(IGitblit gitblit, IdGenerator idGenerator) {
 		this.gitblit = gitblit;
@@ -57,6 +61,15 @@ public class SshCommandFactory implements CommandFactory {
 		int threads = 2;// cfg.getInt("sshd","commandStartThreads", 2);
 		WorkQueue workQueue = new WorkQueue(idGenerator);
 		startExecutor = workQueue.createQueue(threads, "SshCommandStart");
+		destroyExecutor = Executors.newSingleThreadExecutor(
+				new ThreadFactoryBuilder()
+					.setNameFormat("SshCommandDestroy-%s")
+					.setDaemon(true)
+					.build());
+	}
+
+	public void stop() {
+		destroyExecutor.shutdownNow();
 	}
 
 	public RootDispatcher createRootDispatcher(SshDaemonClient client, String commandLine) {
@@ -166,27 +179,23 @@ public class SshCommandFactory implements CommandFactory {
 		}
 
 		private int translateExit(final int rc) {
-			return rc;
-			//
-			// switch (rc) {
-			// case BaseCommand.STATUS_NOT_ADMIN:
-			// return 1;
-			//
-			// case BaseCommand.STATUS_CANCEL:
-			// return 15 /* SIGKILL */;
-			//
-			// case BaseCommand.STATUS_NOT_FOUND:
-			// return 127 /* POSIX not found */;
-			//
-			// default:
-			// return rc;
-			// }
+			switch (rc) {
+			case BaseCommand.STATUS_NOT_ADMIN:
+				return 1;
+
+			case BaseCommand.STATUS_CANCEL:
+				return 15 /* SIGKILL */;
+
+			case BaseCommand.STATUS_NOT_FOUND:
+				return 127 /* POSIX not found */;
 
+			default:
+				return rc;
+			}
 		}
 
 		private void log(final int rc) {
 			if (logged.compareAndSet(false, true)) {
-				// log.onExecute(cmd, rc);
 				logger.info("onExecute: {} exits with: {}", cmd.getClass().getSimpleName(), rc);
 			}
 		}
@@ -196,27 +205,22 @@ public class SshCommandFactory implements CommandFactory {
 			Future<?> future = task.getAndSet(null);
 			if (future != null) {
 				future.cancel(true);
-				// destroyExecutor.execute(new Runnable() {
-				// @Override
-				// public void run() {
-				// onDestroy();
-				// }
-				// });
+				destroyExecutor.execute(new Runnable() {
+					@Override
+					public void run() {
+						onDestroy();
+					}
+				});
 			}
 		}
 
-		@SuppressWarnings("unused")
 		private void onDestroy() {
 			synchronized (this) {
 				if (cmd != null) {
-					// final Context old = sshScope.set(ctx);
 					try {
 						cmd.destroy();
-						// log(BaseCommand.STATUS_CANCEL);
 					} finally {
-						// ctx = null;
 						cmd = null;
-						// sshScope.set(old);
 					}
 				}
 			}
diff --git a/src/main/java/com/gitblit/transport/ssh/git/BaseGitCommand.java b/src/main/java/com/gitblit/transport/ssh/git/BaseGitCommand.java
index 2e4fda5f..fcb06568 100644
--- a/src/main/java/com/gitblit/transport/ssh/git/BaseGitCommand.java
+++ b/src/main/java/com/gitblit/transport/ssh/git/BaseGitCommand.java
@@ -44,6 +44,16 @@ abstract class BaseGitCommand extends BaseCommand {
 	protected Repository repo;
 
 	@Override
+	public void destroy() {
+		super.destroy();
+
+		repositoryResolver = null;
+		receivePackFactory = null;
+		uploadPackFactory = null;
+		repo = null;
+	}
+
+	@Override
 	public void start(final Environment env) {
 		startThread(new RepositoryCommandRunnable() {
 			@Override
diff --git a/src/main/java/com/gitblit/transport/ssh/git/GitDispatcher.java b/src/main/java/com/gitblit/transport/ssh/git/GitDispatcher.java
index fa1dfbd4..a457be9e 100644
--- a/src/main/java/com/gitblit/transport/ssh/git/GitDispatcher.java
+++ b/src/main/java/com/gitblit/transport/ssh/git/GitDispatcher.java
@@ -44,6 +44,15 @@ public class GitDispatcher extends DispatchCommand {
 	}
 
 	@Override
+	public void destroy() {
+		super.destroy();
+
+		repositoryResolver = null;
+		receivePackFactory = null;
+		uploadPackFactory = null;
+	}
+
+	@Override
 	protected void registerCommands(UserModel user) {
 		registerCommand(user, Upload.class);
 		registerCommand(user, Receive.class);