Perform plugin cleanup in the background
When a plugin is stopped, schedule a Plugin Cleaner task to run
1 minute later to try and clean out the garbage and release the
JAR from $site_path/tmp. If the cleaner doesn't succeed, schedule
again for 1 minute later until it does. Each failed clean attempt
backs off by another minute until it tries once every 15 minutes
(4 times per hour).
Plugins that don't clean are probably still linked into the live
object graph somewhere and cannot be garbage collected. This
indicates a memory leak, and the server administrator may need to
restart the server to free up memory. Leaking should only happen
with badly coded plugins that attempt to manually manage their
listener registrations, or start threads they don't terminate.
Change-Id: I0ca8c2c70b6fd9bcfbb7444ea3995210c9d80c2f
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/CleanupHandle.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/CleanupHandle.java
index e18d840..7018a3b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/CleanupHandle.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/CleanupHandle.java
@@ -40,6 +40,8 @@
}
if (!tmpFile.delete() && tmpFile.exists()) {
PluginLoader.log.warn("Cannot delete " + tmpFile.getAbsolutePath());
+ } else {
+ PluginLoader.log.info("Cleaned plugin " + tmpFile.getName());
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginCleanerTask.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginCleanerTask.java
new file mode 100644
index 0000000..7081d70
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginCleanerTask.java
@@ -0,0 +1,100 @@
+// Copyright (C) 2012 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.plugins;
+
+import com.google.gerrit.server.git.WorkQueue;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+@Singleton
+class PluginCleanerTask implements Runnable {
+ private final WorkQueue workQueue;
+ private final PluginLoader loader;
+ private volatile int pending;
+ private Future<?> self;
+ private int attempts;
+ private long start;
+
+ @Inject
+ PluginCleanerTask(WorkQueue workQueue, PluginLoader loader) {
+ this.workQueue = workQueue;
+ this.loader = loader;
+ }
+
+ @Override
+ public void run() {
+ try {
+ for (int t = 0; t < 2 * (attempts + 1); t++) {
+ System.gc();
+ Thread.sleep(50);
+ }
+ } catch (InterruptedException e) {
+ }
+
+ int left = loader.processPendingCleanups();
+ synchronized (this) {
+ pending = left;
+ self = null;
+
+ if (0 < left) {
+ long waiting = System.currentTimeMillis() - start;
+ PluginLoader.log.warn(String.format(
+ "%d plugins still waiting to be reclaimed after %d minutes",
+ pending,
+ TimeUnit.MILLISECONDS.toMinutes(waiting)));
+ attempts = Math.min(attempts + 1, 15);
+ ensureScheduled();
+ } else {
+ attempts = 0;
+ }
+ }
+ }
+
+ @Override
+ public String toString() {
+ int p = pending;
+ if (0 < p) {
+ return String.format("Plugin Cleaner (waiting for %d plugins)", p);
+ }
+ return "Plugin Cleaner";
+ }
+
+ synchronized void clean(int expect) {
+ if (self == null && pending == 0) {
+ start = System.currentTimeMillis();
+ }
+ pending = expect;
+ ensureScheduled();
+ }
+
+ private void ensureScheduled() {
+ if (self == null && 0 < pending) {
+ if (attempts == 1) {
+ self = workQueue.getDefaultQueue().schedule(
+ this,
+ 30,
+ TimeUnit.SECONDS);
+ } else {
+ self = workQueue.getDefaultQueue().schedule(
+ this,
+ attempts + 1,
+ TimeUnit.MINUTES);
+ }
+ }
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java
index 828a88d..324e3de 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
import com.google.inject.Module;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.eclipse.jgit.lib.Config;
@@ -68,12 +69,14 @@
private final Map<String, FileSnapshot> broken;
private final ReferenceQueue<ClassLoader> cleanupQueue;
private final ConcurrentMap<CleanupHandle, Boolean> cleanupHandles;
+ private final Provider<PluginCleanerTask> cleaner;
private final PluginScannerThread scanner;
@Inject
public PluginLoader(SitePaths sitePaths,
PluginGuiceEnvironment pe,
ServerInformationImpl sii,
+ Provider<PluginCleanerTask> pct,
@GerritServerConfig Config cfg) {
pluginsDir = sitePaths.plugins_dir;
dataDir = sitePaths.data_dir;
@@ -84,6 +87,7 @@
broken = Maps.newHashMap();
cleanupQueue = new ReferenceQueue<ClassLoader>();
cleanupHandles = Maps.newConcurrentMap();
+ cleaner = pct;
long checkFrequency = ConfigUtil.getTimeUnit(cfg,
"plugins", null, "checkFrequency",
@@ -110,7 +114,6 @@
File old = new File(pluginsDir, ".last_" + name + ".zip");
File tmp = asTemp(in, ".next_" + name, ".zip", pluginsDir);
- boolean clean = false;
synchronized (this) {
Plugin active = running.get(name);
if (active != null) {
@@ -125,18 +128,13 @@
runPlugin(name, jar, active);
if (active == null) {
log.info(String.format("Installed plugin %s", name));
- } else {
- clean = true;
}
} catch (PluginInstallException e) {
jar.delete();
throw e;
}
- }
- if (clean) {
- System.gc();
- processPendingCleanups();
+ cleanInBackground();
}
}
@@ -166,7 +164,6 @@
}
public void disablePlugins(Set<String> names) {
- boolean clean = false;
synchronized (this) {
for (String name : names) {
Plugin active = running.get(name);
@@ -180,12 +177,8 @@
active.stop();
running.remove(name);
- clean = true;
}
- }
- if (clean) {
- System.gc();
- processPendingCleanups();
+ cleanInBackground();
}
}
@@ -193,7 +186,7 @@
public synchronized void start() {
log.info("Loading plugins from " + pluginsDir.getAbsolutePath());
srvInfoImpl.state = ServerInformation.State.STARTUP;
- rescan(false);
+ rescan();
srvInfoImpl.state = ServerInformation.State.RUNNING;
if (scanner != null) {
scanner.start();
@@ -207,26 +200,18 @@
}
srvInfoImpl.state = ServerInformation.State.SHUTDOWN;
synchronized (this) {
- boolean clean = !running.isEmpty();
for (Plugin p : running.values()) {
p.stop();
}
running.clear();
broken.clear();
- if (clean) {
+ if (cleanupHandles.size() > running.size()) {
System.gc();
processPendingCleanups();
}
}
}
- public void rescan(boolean forceCleanup) {
- if (rescanImp() || forceCleanup) {
- System.gc();
- processPendingCleanups();
- }
- }
-
public void reload(List<String> names)
throws InvalidPluginException, PluginInstallException {
synchronized (this) {
@@ -256,14 +241,14 @@
throw e;
}
}
- System.gc();
- processPendingCleanups();
+
+ cleanInBackground();
}
}
- private synchronized boolean rescanImp() {
+ public synchronized void rescan() {
List<File> jars = scanJarsInPluginsDirectory();
- boolean clean = stopRemovedPlugins(jars);
+ stopRemovedPlugins(jars);
for (File jar : jars) {
String name = nameOf(jar);
@@ -285,14 +270,13 @@
runPlugin(name, jar, active);
if (active == null) {
log.info(String.format("Loaded plugin %s", name));
- } else {
- clean = true;
}
} catch (PluginInstallException e) {
log.warn(String.format("Cannot load plugin %s", name), e.getCause());
}
}
- return clean;
+
+ cleanInBackground();
}
private void runPlugin(String name, File jar, Plugin oldPlugin)
@@ -322,7 +306,7 @@
}
}
- private boolean stopRemovedPlugins(List<File> jars) {
+ private void stopRemovedPlugins(List<File> jars) {
Set<String> unload = Sets.newHashSet(running.keySet());
for (File jar : jars) {
unload.remove(nameOf(jar));
@@ -331,15 +315,22 @@
log.info(String.format("Unloading plugin %s", name));
running.remove(name).stop();
}
- return !unload.isEmpty();
}
- private synchronized void processPendingCleanups() {
+ synchronized int processPendingCleanups() {
CleanupHandle h;
while ((h = (CleanupHandle) cleanupQueue.poll()) != null) {
h.cleanup();
cleanupHandles.remove(h);
}
+ return Math.max(0, cleanupHandles.size() - running.size());
+ }
+
+ private void cleanInBackground() {
+ int cnt = Math.max(0, cleanupHandles.size() - running.size());
+ if (0 < cnt) {
+ cleaner.get().clean(cnt);
+ }
}
private static String nameOf(File jar) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginModule.java
index 5f5b8e3..ab7dc3c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginModule.java
@@ -23,8 +23,10 @@
bind(ServerInformationImpl.class);
bind(ServerInformation.class).to(ServerInformationImpl.class);
+ bind(PluginCleanerTask.class);
bind(PluginGuiceEnvironment.class);
bind(PluginLoader.class);
+
bind(CopyConfigModule.class);
listener().to(PluginLoader.class);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginScannerThread.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginScannerThread.java
index b2e3fed..a484c5d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginScannerThread.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginScannerThread.java
@@ -38,7 +38,7 @@
}
} catch (InterruptedException e) {
}
- loader.rescan(false);
+ loader.rescan();
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PluginReloadCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PluginReloadCommand.java
index e01c6d8..d60465c 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PluginReloadCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PluginReloadCommand.java
@@ -37,7 +37,7 @@
@Override
protected void run() throws UnloggedFailure {
if (names == null || names.isEmpty()) {
- loader.rescan(true);
+ loader.rescan();
} else {
try {
loader.reload(names);
