Skip to content

Commit 5fff544

Browse files
authored
Merge branch 'master' into refacrot-lazy-loading-part2
2 parents 39e329d + c781b99 commit 5fff544

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+532
-453
lines changed

bom/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ THE SOFTWARE.
6363
<dependency>
6464
<groupId>org.springframework</groupId>
6565
<artifactId>spring-framework-bom</artifactId>
66-
<version>6.2.9</version>
66+
<version>6.2.10</version>
6767
<type>pom</type>
6868
<scope>import</scope>
6969
</dependency>

core/pom.xml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,6 @@ THE SOFTWARE.
117117
</exclusion>
118118
</exclusions>
119119
</dependency>
120-
<dependency>
121-
<groupId>com.infradna.tool</groupId>
122-
<artifactId>bridge-method-annotation</artifactId>
123-
<version>${bridge-method-injector.version}</version>
124-
</dependency>
125120
<dependency>
126121
<groupId>com.sun.xml.txw2</groupId>
127122
<artifactId>txw2</artifactId>
@@ -168,6 +163,11 @@ THE SOFTWARE.
168163
<groupId>commons-lang</groupId>
169164
<artifactId>commons-lang</artifactId>
170165
</dependency>
166+
<dependency>
167+
<groupId>io.jenkins.tools</groupId>
168+
<artifactId>bridge-method-annotation</artifactId>
169+
<version>${bridge-method-injector.version}</version>
170+
</dependency>
171171
<dependency>
172172
<!-- needed by Jelly -->
173173
<groupId>jakarta.servlet.jsp.jstl</groupId>
@@ -568,7 +568,7 @@ THE SOFTWARE.
568568
</executions>
569569
</plugin>
570570
<plugin>
571-
<groupId>com.infradna.tool</groupId>
571+
<groupId>io.jenkins.tools</groupId>
572572
<artifactId>bridge-method-injector</artifactId>
573573
<executions>
574574
<execution>

core/src/main/java/hudson/ClassicPluginStrategy.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import hudson.model.Hudson;
3434
import hudson.util.CyclicGraphDetector;
3535
import hudson.util.CyclicGraphDetector.CycleDetectedException;
36+
import hudson.util.DelegatingClassLoader;
3637
import hudson.util.IOUtils;
3738
import hudson.util.MaskingClassLoader;
3839
import java.io.File;
@@ -559,7 +560,7 @@ private static void unzipExceptClasses(File archive, File destDir, Project prj)
559560
/**
560561
* Used to load classes from dependency plugins.
561562
*/
562-
static final class DependencyClassLoader extends ClassLoader {
563+
static final class DependencyClassLoader extends DelegatingClassLoader {
563564
/**
564565
* This classloader is created for this plugin. Useful during debugging.
565566
*/
@@ -574,10 +575,6 @@ static final class DependencyClassLoader extends ClassLoader {
574575
*/
575576
private volatile List<PluginWrapper> transitiveDependencies;
576577

577-
static {
578-
registerAsParallelCapable();
579-
}
580-
581578
DependencyClassLoader(ClassLoader parent, File archive, List<Dependency> dependencies, PluginManager pluginManager) {
582579
super("dependency ClassLoader for " + archive.getPath(), parent);
583580
this._for = archive;

core/src/main/java/hudson/PluginManager.java

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@
6060
import hudson.security.PermissionScope;
6161
import hudson.util.CyclicGraphDetector;
6262
import hudson.util.CyclicGraphDetector.CycleDetectedException;
63+
import hudson.util.DelegatingClassLoader;
64+
import hudson.util.ExistenceCheckingClassLoader;
6365
import hudson.util.FormValidation;
6466
import hudson.util.PersistedList;
6567
import hudson.util.Retrier;
@@ -218,6 +220,14 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
218220
*/
219221
/* private final */ static int CHECK_UPDATE_ATTEMPTS;
220222

223+
/**
224+
* Class name prefixes to skip in the class loading
225+
*/
226+
private static final String[] CLASS_PREFIXES_TO_SKIP = {
227+
"SimpleTemplateScript", // cf. groovy.text.SimpleTemplateEngine
228+
"groovy.tmp.templates.GStringTemplateScript", // Leaks on classLoader in some cases, see JENKINS-75879
229+
};
230+
221231
static {
222232
try {
223233
// Secure initialization
@@ -2392,25 +2402,35 @@ public <T> T of(String key, Class<T> type, Supplier<T> func) {
23922402
/**
23932403
* {@link ClassLoader} that can see all plugins.
23942404
*/
2395-
public static final class UberClassLoader extends ClassLoader {
2405+
public static final class UberClassLoader extends DelegatingClassLoader {
23962406
private final List<PluginWrapper> activePlugins;
23972407

23982408
/** Cache of loaded, or known to be unloadable, classes. */
23992409
private final ConcurrentMap<String, Optional<Class<?>>> loaded = new ConcurrentHashMap<>();
24002410

2401-
static {
2402-
registerAsParallelCapable();
2403-
}
2404-
2411+
/**
2412+
* The servlet container's {@link ClassLoader} (the parent of Jenkins core) is
2413+
* parallel-capable and maintains its own growing {@link Map} of {@link
2414+
* ClassLoader#getClassLoadingLock} objects per class name for every load attempt (including
2415+
* misses), and we cannot override this behavior. Wrap the servlet container {@link
2416+
* ClassLoader} in {@link ExistenceCheckingClassLoader} to avoid calling {@link
2417+
* ClassLoader#getParent}'s {@link ClassLoader#loadClass(String, boolean)} at all for misses
2418+
* by first checking if the resource exists. If the resource does not exist, we immediately
2419+
* throw {@link ClassNotFoundException}. As a result, the servlet container's {@link
2420+
* ClassLoader} is never asked to try and fail, and it never creates/retains lock objects
2421+
* for those misses.
2422+
*/
24052423
public UberClassLoader(List<PluginWrapper> activePlugins) {
2406-
super("UberClassLoader", PluginManager.class.getClassLoader());
2424+
super("UberClassLoader", new ExistenceCheckingClassLoader(PluginManager.class.getClassLoader()));
24072425
this.activePlugins = activePlugins;
24082426
}
24092427

24102428
@Override
24112429
protected Class<?> findClass(String name) throws ClassNotFoundException {
2412-
if (name.startsWith("SimpleTemplateScript")) { // cf. groovy.text.SimpleTemplateEngine
2413-
throw new ClassNotFoundException("ignoring " + name);
2430+
for (String namePrefixToSkip : CLASS_PREFIXES_TO_SKIP) {
2431+
if (name.startsWith(namePrefixToSkip)) {
2432+
throw new ClassNotFoundException("ignoring " + name);
2433+
}
24142434
}
24152435
return loaded.computeIfAbsent(name, this::computeValue).orElseThrow(() -> new ClassNotFoundException(name));
24162436
}

core/src/main/java/hudson/diagnosis/ReverseProxySetupMonitor.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ public boolean isActivated() {
6666
return true;
6767
}
6868

69+
@Override
70+
public boolean isActivationFake() {
71+
return true;
72+
}
73+
6974
@Restricted(DoNotUse.class) // WebOnly
7075
@RestrictedSince("2.235")
7176
public HttpResponse doTest(StaplerRequest2 request, @QueryParameter boolean testWithContext) {

core/src/main/java/hudson/model/AdministrativeMonitor.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,11 @@ public boolean isEnabled() {
160160
*/
161161
public abstract boolean isActivated();
162162

163+
@Restricted(NoExternalUse.class)
164+
public boolean isActivationFake() {
165+
return false;
166+
}
167+
163168
/**
164169
* Returns true if this monitor is security related.
165170
*
@@ -186,8 +191,7 @@ public void doDisable(StaplerRequest2 req, StaplerResponse2 rsp) throws IOExcept
186191
* By default {@link Jenkins#ADMINISTER}, but {@link Jenkins#SYSTEM_READ} or {@link Jenkins#MANAGE} are also supported.
187192
* <p>
188193
* Changing this permission check to return {@link Jenkins#SYSTEM_READ} will make the active
189-
* administrative monitor appear on {@code manage.jelly} and on the globally visible
190-
* {@link jenkins.management.AdministrativeMonitorsDecorator} to users without Administer permission.
194+
* administrative monitor appear on {@link ManageJenkinsAction} to users without Administer permission.
191195
* {@link #doDisable(StaplerRequest2, StaplerResponse2)} will still always require Administer permission.
192196
* </p>
193197
* <p>

core/src/main/java/hudson/model/Computer.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,19 @@ public final boolean isOnline() {
625625
return !isOffline();
626626
}
627627

628+
/**
629+
* {@inheritDoc}
630+
* <p>
631+
* Uses {@link #getChannel()} to check the connection.
632+
* A connected agent may still be offline for scheduling if marked temporarily offline.
633+
* @return {@code true} if the agent is connected, {@code false} otherwise.
634+
* @see #isOffline()
635+
*/
636+
@Override
637+
public boolean isConnected() {
638+
return getChannel() != null;
639+
}
640+
628641
/**
629642
* This method is called to determine whether manual launching of the agent is allowed at this point in time.
630643
* @return {@code true} if manual launching of the agent is allowed at this point in time.

core/src/main/java/hudson/model/ManageJenkinsAction.java

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@
2626

2727
import hudson.Extension;
2828
import hudson.Util;
29+
import hudson.util.HudsonIsLoading;
30+
import hudson.util.HudsonIsRestarting;
2931
import java.io.IOException;
30-
import java.util.Collection;
31-
import java.util.Collections;
32-
import java.util.Optional;
33-
import jenkins.management.AdministrativeMonitorsDecorator;
32+
import java.util.logging.Level;
33+
import java.util.logging.Logger;
3434
import jenkins.management.Badge;
3535
import jenkins.model.Jenkins;
3636
import jenkins.model.ModelObjectWithContextMenu;
@@ -50,6 +50,9 @@
5050
*/
5151
@Extension(ordinal = 998) @Symbol("manageJenkins")
5252
public class ManageJenkinsAction implements RootAction, StaplerFallback, ModelObjectWithContextMenu {
53+
54+
private static final Logger LOGGER = Logger.getLogger(ManageJenkinsAction.class.getName());
55+
5356
@Override
5457
public String getIconFileName() {
5558
if (Jenkins.get().hasAnyPermission(Jenkins.MANAGE, Jenkins.SYSTEM_READ))
@@ -98,28 +101,36 @@ public void addContextMenuItem(ContextMenu menu, String url, String icon, String
98101
menu.add("manage/" + url, icon, iconXml, text, post, requiresConfirmation, badge, message);
99102
}
100103

104+
/** Unlike {@link Jenkins#getActiveAdministrativeMonitors} this checks for activation lazily. */
101105
@Override
102106
public Badge getBadge() {
103-
Jenkins jenkins = Jenkins.get();
104-
AdministrativeMonitorsDecorator decorator = jenkins.getExtensionList(PageDecorator.class)
105-
.get(AdministrativeMonitorsDecorator.class);
106-
107-
if (decorator == null) {
107+
if (!(AdministrativeMonitor.hasPermissionToDisplay())) {
108108
return null;
109109
}
110110

111-
Collection<AdministrativeMonitor> activeAdministrativeMonitors = Optional.ofNullable(decorator.getMonitorsToDisplay()).orElse(Collections.emptyList());
112-
boolean anySecurity = activeAdministrativeMonitors.stream().anyMatch(AdministrativeMonitor::isSecurity);
113-
114-
if (activeAdministrativeMonitors.isEmpty()) {
111+
var app = Jenkins.get().getServletContext().getAttribute("app");
112+
if (app instanceof HudsonIsLoading || app instanceof HudsonIsRestarting) {
115113
return null;
116114
}
117115

118-
int size = activeAdministrativeMonitors.size();
119-
String tooltip = size > 1 ? Messages.ManageJenkinsAction_notifications(size) : Messages.ManageJenkinsAction_notification(size);
116+
if (Jenkins.get().administrativeMonitors.stream().anyMatch(m -> m.isSecurity() && isActive(m))) {
117+
return new Badge("1+", Messages.ManageJenkinsAction_notifications(),
118+
Badge.Severity.DANGER);
119+
} else if (Jenkins.get().administrativeMonitors.stream().anyMatch(m -> !m.isSecurity() && isActive(m))) {
120+
return new Badge("1+", Messages.ManageJenkinsAction_notifications(),
121+
Badge.Severity.WARNING);
122+
} else {
123+
return null;
124+
}
125+
}
120126

121-
return new Badge(String.valueOf(size),
122-
tooltip,
123-
anySecurity ? Badge.Severity.DANGER : Badge.Severity.WARNING);
127+
private static boolean isActive(AdministrativeMonitor m) {
128+
try {
129+
return !m.isActivationFake() && m.hasRequiredPermission() && m.isEnabled() && m.isActivated();
130+
} catch (Throwable x) {
131+
LOGGER.log(Level.WARNING, null, x);
132+
return false;
133+
}
124134
}
135+
125136
}
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package hudson.util;
2+
3+
import java.util.Objects;
4+
import org.kohsuke.accmod.Restricted;
5+
import org.kohsuke.accmod.restrictions.NoExternalUse;
6+
7+
/**
8+
* A {@link ClassLoader} that does not define any classes itself but delegates class loading to
9+
* other class loaders to avoid the JDK's per-class-name locking and lock retention.
10+
*
11+
* <p>This class first attempts to load classes via its {@link ClassLoader#getParent} class loader,
12+
* then falls back to {@link ClassLoader#findClass} to allow for custom delegation logic.
13+
*
14+
* <p>In a parallel-capable {@link ClassLoader}, the JDK maintains a per-name lock object
15+
* indefinitely. In Jenkins, many class loading misses across many loaders can accumulate hundreds
16+
* of thousands of such locks, retaining significant memory. This loader never defines classes and
17+
* bypasses {@link ClassLoader}'s default {@code loadClass} locking; it delegates to the parent
18+
* first and then to {@code findClass} for custom delegation.
19+
*
20+
* <p>The actual defining loader (parent or a delegate) still performs the necessary synchronization
21+
* and class definition. A runtime guard ({@link #verify}) throws if this loader ever becomes the
22+
* defining loader.
23+
*
24+
* <p>Subclasses must not call {@code defineClass}; implement delegation in {@code findClass} if
25+
* needed and do not mark subclasses as parallel-capable.
26+
*
27+
* @author Dmytro Ukhlov
28+
*/
29+
@Restricted(NoExternalUse.class)
30+
public abstract class DelegatingClassLoader extends ClassLoader {
31+
protected DelegatingClassLoader(String name, ClassLoader parent) {
32+
super(name, Objects.requireNonNull(parent));
33+
}
34+
35+
public DelegatingClassLoader(ClassLoader parent) {
36+
super(Objects.requireNonNull(parent));
37+
}
38+
39+
/**
40+
* Parent-first delegation without synchronizing on {@link #getClassLoadingLock(String)}. This
41+
* prevents creation/retention of per-name lock objects in a loader that does not define
42+
* classes. The defining loader downstream still serializes class definition as required.
43+
*
44+
* @param name The binary name of the class
45+
* @param resolve If {@code true} then resolve the class
46+
* @return The resulting {@link Class} object
47+
* @throws ClassNotFoundException If the class could not be found
48+
*/
49+
@Override
50+
protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
51+
Class<?> c = null;
52+
try {
53+
c = getParent().loadClass(name);
54+
} catch (ClassNotFoundException e) {
55+
// ClassNotFoundException thrown if class not found
56+
// from the non-null parent class loader
57+
}
58+
59+
if (c == null) {
60+
// If still not found, then invoke findClass in order
61+
// to find the class.
62+
c = findClass(name);
63+
}
64+
65+
verify(c);
66+
if (resolve) {
67+
resolveClass(c);
68+
}
69+
return c;
70+
}
71+
72+
/**
73+
* Safety check to ensure this delegating loader never becomes the defining loader.
74+
*
75+
* <p>Fails fast if a subclass erroneously defines a class here, which would violate the
76+
* delegation-only contract and could reintroduce locking/retention issues.
77+
*/
78+
private void verify(Class<?> clazz) {
79+
if (clazz.getClassLoader() == this) {
80+
throw new IllegalStateException("DelegatingClassLoader must not be the defining loader: " + clazz.getName());
81+
}
82+
}
83+
}

0 commit comments

Comments
 (0)