Merge pull request #933 from dwnusbaum/cleanupglobalclassvalue-npes

Prevent NPEs in `CpsFlowExecution.cleanUpGlobalClassValue` to make Pipeline cleanup more robust
jenkinsci · Sep 17, 2024 · 0767b4b · 0767b4b
2 parents e48ee2c + a75664a
commit 0767b4b
Showing 1 changed file with 9 additions and 46 deletions.
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java
@@ -1437,7 +1437,6 @@ private static void cleanUpLoader(ClassLoader loader, Set<ClassLoader> encounter
             if (encounteredClasses.add(clazz)) {
                 LOGGER.finer(() -> "found " + clazz.getName());
                 Introspector.flushFromCaches(clazz);
-                cleanUpGlobalClassSet(clazz);
                 cleanUpClassHelperCache(clazz);
                 cleanUpLoader(clazz.getClassLoader(), encounteredLoaders, encounteredClasses);
             }
@@ -1464,19 +1463,15 @@ private static void cleanUpGlobalClassValue(@NonNull ClassLoader loader) throws
         Class<?> entryC = Class.forName("org.codehaus.groovy.util.AbstractConcurrentMapBase$Entry");
         Method getValueM = entryC.getMethod("getValue");
         List<Class<?>> toRemove = new ArrayList<>(); // not sure if it is safe against ConcurrentModificationException or not
-        try {
-            Field classRefF = classInfoC.getDeclaredField("classRef"); // 2.4.8+
-            classRefF.setAccessible(true);
-            for (Object entry : entries) {
-                Object value = getValueM.invoke(entry);
-                toRemove.add(((WeakReference<Class<?>>) classRefF.get(value)).get());
-            }
-        } catch (NoSuchFieldException x) {
-            Field klazzF = classInfoC.getDeclaredField("klazz"); // 2.4.7-
-            klazzF.setAccessible(true);
-            for (Object entry : entries) {
-                Object value = getValueM.invoke(entry);
-                toRemove.add((Class) klazzF.get(value));
+        Field classRefF = classInfoC.getDeclaredField("classRef"); // 2.4.8+
+        classRefF.setAccessible(true);
+        for (Object entry : entries) {
+            Object classInfo = getValueM.invoke(entry);
+            if (classInfo != null) {
+                Class<?> clazz = ((WeakReference<Class<?>>) classRefF.get(classInfo)).get();
+                if (clazz != null) {
+                    toRemove.add(clazz);
+                }
             }
         }
         Iterator<Class<?>> it = toRemove.iterator();
@@ -1496,38 +1491,6 @@ private static void cleanUpGlobalClassValue(@NonNull ClassLoader loader) throws
         }
     }
 
-    private static void cleanUpGlobalClassSet(@NonNull Class<?> clazz) throws Exception {
-        Class<?> classInfoC = Class.forName("org.codehaus.groovy.reflection.ClassInfo"); // or just ClassInfo.class, but unclear whether this will always be there
-        Field globalClassSetF = classInfoC.getDeclaredField("globalClassSet");
-        globalClassSetF.setAccessible(true);
-        Object globalClassSet = globalClassSetF.get(null);
-        try {
-            classInfoC.getDeclaredField("classRef");
-            return; // 2.4.8+, nothing to do here (classRef is weak anyway)
-        } catch (NoSuchFieldException x2) {} // 2.4.7-
-        // Cannot just call .values() since that returns a copy.
-        Field itemsF = globalClassSet.getClass().getDeclaredField("items");
-        itemsF.setAccessible(true);
-        Object items = itemsF.get(globalClassSet);
-        Method iteratorM = items.getClass().getMethod("iterator");
-        Field klazzF = classInfoC.getDeclaredField("klazz");
-        klazzF.setAccessible(true);
-        synchronized (items) {
-            Iterator<?> iterator = (Iterator) iteratorM.invoke(items);
-            while (iterator.hasNext()) {
-                Object classInfo = iterator.next();
-                if (classInfo == null) {
-                    LOGGER.finer("JENKINS-41945: ignoring null ClassInfo from ManagedLinkedList.Iter.next");
-                    continue;
-                }
-                if (klazzF.get(classInfo) == clazz) {
-                    iterator.remove();
-                    LOGGER.log(Level.FINER, "cleaning up {0} from GlobalClassSet", clazz.getName());
-                }
-            }
-        }
-    }
-
     private static void cleanUpClassHelperCache(@NonNull Class<?> clazz) throws Exception {
         Field classCacheF = Class.forName("org.codehaus.groovy.ast.ClassHelper$ClassHelperCache").getDeclaredField("classCache");
         classCacheF.setAccessible(true);