Merge changes Iaa8658bd,Ifdae88ec into stable-2.4
* changes:
Fix deadlock on destroy of CommandFactoryProvider.
Use AtomicBoolean for "logged" in CommandFactoryProvider.
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java
index fb17169..66e6add 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java
@@ -14,6 +14,8 @@
package com.google.gerrit.sshd;
+import com.google.common.util.concurrent.Atomics;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.sshd.SshScope.Context;
@@ -36,6 +38,11 @@
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
/**
* Creates a CommandFactory using commands registered by {@link CommandModule}.
@@ -46,7 +53,8 @@
private final DispatchCommandProvider dispatcher;
private final SshLog log;
- private final Executor startExecutor;
+ private final ScheduledExecutorService startExecutor;
+ private final Executor destroyExecutor;
@Inject
CommandFactoryProvider(
@@ -58,6 +66,11 @@
int threads = cfg.getInt("sshd","commandStartThreads", 2);
startExecutor = workQueue.createQueue(threads, "SshCommandStart");
+ destroyExecutor = Executors.newSingleThreadExecutor(
+ new ThreadFactoryBuilder()
+ .setNameFormat("SshCommandDestroy-%s")
+ .setDaemon(true)
+ .build());
}
@Override
@@ -79,11 +92,14 @@
private Environment env;
private Context ctx;
private DispatchCommand cmd;
- private boolean logged;
+ private final AtomicBoolean logged;
+ private final AtomicReference<Future<?>> task;
Trampoline(final String cmdLine) {
commandLine = cmdLine;
argv = split(cmdLine);
+ logged = new AtomicBoolean();
+ task = Atomics.newReference();
}
public void setInputStream(final InputStream in) {
@@ -110,7 +126,7 @@
public void start(final Environment env) throws IOException {
this.env = env;
final Context ctx = this.ctx;
- startExecutor.execute(new Runnable() {
+ task.set(startExecutor.submit(new Runnable() {
public void run() {
try {
onStart();
@@ -124,7 +140,7 @@
public String toString() {
return "start (user " + ctx.getSession().getUsername() + ")";
}
- });
+ }));
}
private void onStart() throws IOException {
@@ -173,16 +189,26 @@
}
private void log(final int rc) {
- synchronized (this) {
- if (!logged) {
- log.onExecute(rc);
- logged = true;
- }
+ if (logged.compareAndSet(false, true)) {
+ log.onExecute(rc);
}
}
@Override
public void destroy() {
+ Future<?> future = task.getAndSet(null);
+ if (future != null) {
+ future.cancel(true);
+ destroyExecutor.execute(new Runnable() {
+ @Override
+ public void run() {
+ onDestroy();
+ }
+ });
+ }
+ }
+
+ private void onDestroy() {
synchronized (this) {
if (cmd != null) {
final Context old = SshScope.set(ctx);
