[FIXED JENKINS-33681] Plugin filters were failing to be removed and b…

…locking restart
jenkinsci · Mar 21, 2016 · a5febd7 · a5febd7 · oleg-nenashev · Mar 22, 2016
1 parent 60ba6ee
commit a5febd7
Showing 1 changed file with 13 additions and 4 deletions.
diff --git a/core/src/main/java/hudson/util/PluginServletFilter.java b/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);
             }
         }
     }