opendev/gerrit
Code Issues Proposed changes

Try to stop plugin if enabling it failed

Browse Source 
Without this, a plugin failing to load might break other unrelated
plugins. This occurs because a Guice environment is created and a
serverManager is created but never started.

This should only be triggered when a bogus plugin is installed, which
is more likely to occur during plugin development.

Also initialize the fields outside of the constructor.

Change-Id: I0bcbe331697e320430711a776357800f1f69c8a3
This commit is contained in:
Maxime Guerreiro
2018-04-16 13:13:43 +00:00
parent aa6bf8ef69
commit f13ee1e78a
1 changed files with 11 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
java/com/google/gerrit/server/plugins/PluginLoader.java
View File

@@ -50,7 +50,6 @@ import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -79,11 +78,11 @@ public class PluginLoader implements LifecycleListener {
private final PluginGuiceEnvironment env;
private final ServerInformationImpl srvInfoImpl;
private final PluginUser.Factory pluginUserFactory;
private final ConcurrentMap<String, Plugin> running;
private final ConcurrentMap<String, Plugin> disabled;
private final Map<String, FileSnapshot> broken;
private final Map<Plugin, CleanupHandle> cleanupHandles;
private final Queue<Plugin> toCleanup;
private final ConcurrentMap<String, Plugin> running = Maps.newConcurrentMap();
private final ConcurrentMap<String, Plugin> disabled = Maps.newConcurrentMap();
private final Map<String, FileSnapshot> broken = Maps.newHashMap();
private final Map<Plugin, CleanupHandle> cleanupHandles = Maps.newConcurrentMap();
private final Queue<Plugin> toCleanup = new ArrayDeque<>();
private final Provider<PluginCleanerTask> cleaner;
private final PluginScannerThread scanner;
private final Provider<String> urlProvider;
@@ -108,11 +107,6 @@ public class PluginLoader implements LifecycleListener {
env = pe;
srvInfoImpl = sii;
pluginUserFactory = puf;
running = Maps.newConcurrentMap();
disabled = Maps.newConcurrentMap();
broken = new HashMap<>();
toCleanup = new ArrayDeque<>();
cleanupHandles = Maps.newConcurrentMap();
cleaner = pct;
urlProvider = provider;
persistentCacheFactory = cacheFactory;
@@ -495,7 +489,12 @@ public class PluginLoader implements LifecycleListener {
unloadPlugin(oldPlugin);
}
if (!newPlugin.isDisabled()) {
newPlugin.start(env);
try {
newPlugin.start(env);
} catch (Throwable e) {
newPlugin.stop(env);
throw e;
}
}
if (reload) {
env.onReloadPlugin(oldPlugin, newPlugin);