Skip to content
Permalink
Browse files
[FIXED JENKINS-33681] Plugin filters were failing to be removed and b…
…locking restart
  • Loading branch information
stephenc committed Mar 21, 2016
1 parent 60ba6ee commit a5febd7666fd78542d45428505cc62c067315c43
Showing with 13 additions and 4 deletions.
  1. +13 −4 core/src/main/java/hudson/util/PluginServletFilter.java
@@ -25,6 +25,7 @@

import hudson.ExtensionPoint;
import hudson.security.SecurityRealm;
import java.util.ArrayList;
import jenkins.model.Jenkins;

import javax.annotation.CheckForNull;
@@ -148,18 +149,26 @@ public void destroy() {
public static void cleanUp() {
PluginServletFilter instance = getInstance(Jenkins.getInstance().servletContext);
if (instance != null) {
for (Iterator<Filter> iterator = instance.list.iterator(); iterator.hasNext(); ) {
Filter f = iterator.next();
// While we could rely on the current implementation of list being a CopyOnWriteArrayList
// safer to just take an explicit copy of the list and operate on the copy
for (Filter f: new ArrayList<>(instance.list)) {
instance.list.remove(f);
// remove from the list even if destroy() fails as a failed destroy is still a destroy
try {
f.destroy();
} catch (RuntimeException e) {
LOGGER.log(Level.WARNING, "Filter " + f + " propagated an exception from its destroy method", e);
LOGGER.log(Level.WARNING, "Filter " + f + " propagated an exception from its destroy method",
e);
} catch (Error e) {
throw e; // we are not supposed to catch errors, don't log as could be an OOM
} catch (Throwable e) {
LOGGER.log(Level.SEVERE, "Filter " + f + " propagated an exception from its destroy method", e);
}
iterator.remove();
}
// if some fool adds a filter while we are terminating, we should just log the fact
if (!instance.list.isEmpty()) {
LOGGER.log(Level.SEVERE, "The following filters appear to have been added during clean up: {0}",
instance.list);
}
}
}

2 comments on commit a5febd7

@oleg-nenashev
Copy link
Member

@oleg-nenashev oleg-nenashev commented on a5febd7 Mar 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daniel-beck
Copy link
Member

@daniel-beck daniel-beck commented on a5febd7 Mar 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oleg-nenashev This commit isn't on 2.0 branch yet, but it may well prevent JENKINS-33729 from happening AFAIU.

Please sign in to comment.