From c57af86955c2c799930a93cd955ed7f98c7dc504 Mon Sep 17 00:00:00 2001
From: Stephen Connolly <stephen.alan.connolly@gmail.com>
Date: Mon, 21 Mar 2016 14:21:44 +0000
Subject: [PATCH] [FIXED JENKINS-33681] Plugin filters were failing to be
 removed and blocking restart

(cherry picked from commit a5febd7666fd78542d45428505cc62c067315c43)
---
 .../java/hudson/util/PluginServletFilter.java   | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/core/src/main/java/hudson/util/PluginServletFilter.java b/core/src/main/java/hudson/util/PluginServletFilter.java
index 5653ff5d41..dd0069449d 100644
--- a/core/src/main/java/hudson/util/PluginServletFilter.java
+++ b/core/src/main/java/hudson/util/PluginServletFilter.java
@@ -25,6 +25,7 @@ package hudson.util;
 
 import hudson.ExtensionPoint;
 import hudson.security.SecurityRealm;
+import java.util.ArrayList;
 import jenkins.model.Jenkins;
 
 import javax.annotation.CheckForNull;
@@ -148,18 +149,26 @@ public class PluginServletFilter implements Filter, ExtensionPoint {
     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);
             }
         }
     }
-- 
GitLab