[JENKINS-45755] - Make JARCache nullable in Channel

jenkinsci · Sep 26, 2017 · 8f55005 · 8f55005
1 parent a5264a5
commit 8f55005
Show file tree

Hide file tree

Showing 10 changed files with 90 additions and 20 deletions.
diff --git a/src/main/java/hudson/remoting/Channel.java b/src/main/java/hudson/remoting/Channel.java
@@ -317,7 +317,12 @@ public class Channel implements VirtualChannel, IChannel, Closeable {
      */
     /*package*/ final ClassLoader baseClassLoader;
 
-    @Nonnull
+    /**
+     * JAR Resolution Cache.
+     * Can be {@code null} if caching disabled for this channel.
+     * In such case some classloading operations may be rejected.
+     */
+    @CheckForNull
     private JarCache jarCache;
 
     /*package*/ final JarLoaderImpl jarLoader;
@@ -515,12 +520,10 @@ protected Channel(@Nonnull ChannelBuilder settings, @Nonnull CommandTransport tr
         this.underlyingOutput = transport.getUnderlyingStream();
 
         // JAR Cache resolution
-        JarCache effectiveJarCache = settings.getJarCache();
-        if (effectiveJarCache == null) {
-            effectiveJarCache = JarCache.getDefault();
-            logger.log(Level.CONFIG, "Using the default JAR Cache: {0}", effectiveJarCache);
+        this.jarCache = settings.getJarCache();
+        if (this.jarCache == null) {
+            logger.log(Level.CONFIG, "JAR Cache is not defined for channel {0}", name);
         }
-        this.jarCache = effectiveJarCache;
 
         this.baseClassLoader = settings.getBaseLoader();
         this.classFilter = settings.getClassFilter();
@@ -838,9 +841,12 @@ public boolean preloadJar(ClassLoader local, URL... jars) throws IOException, In
 
     /**
      * If this channel is built with jar file caching, return the object that manages this cache.
+     * @return JAR Cache object. {@code null} if JAR caching is disabled
      * @since 2.24
+     * @since 3.10 JAR Cache is Nonnull
+     * @since 3.12 JAR Cache made nullable again due to <a href="https://issues.jenkins-ci.org/browse/JENKINS-45755">JENKINS-45755</a>
      */
-    @Nonnull
+    @CheckForNull
     public JarCache getJarCache() {
         return jarCache;
     }
@@ -851,6 +857,9 @@ public JarCache getJarCache() {
      *
      * So to best avoid performance loss due to race condition, please set a JarCache in the constructor,
      * unless your call sequence guarantees that you call this method before remote classes are loaded.
+     *
+     * @param jarCache New JAR Cache to be used.
+     *                 Cannot be {@code null}, JAR Cache disabling on a running channel is not supported.
      * @since 2.24
      */
     public void setJarCache(@Nonnull JarCache jarCache) {

diff --git a/src/main/java/hudson/remoting/ChannelBuilder.java b/src/main/java/hudson/remoting/ChannelBuilder.java
@@ -192,18 +192,47 @@ public boolean isRemoteClassLoadingAllowed() {
 
     /**
      * Sets the JAR cache storage.
-     * @param jarCache JAR Cache to be used. If {@code null}, a default value will be used by the {@link Channel}
+     * @param jarCache JAR Cache to be used. If a deprecated {@code null} value is passed,
+     *                 the behavior will be determined by the {@link Channel} implementation.
      * @return {@code this}
+     * @since 2.38
+     * @since 3.12 {@code null} parameter value is deprecated.
+     *        {@link #withoutJarCache()} or {@link #withJarCacheOrDefault(JarCache)} should be used instead.
      */
-    public ChannelBuilder withJarCache(@CheckForNull JarCache jarCache) {
+    public ChannelBuilder withJarCache(@Nonnull JarCache jarCache) {
         this.jarCache = jarCache;
         return this;
     }
 
+    /**
+     * Sets the JAR cache storage.
+     * @param jarCache JAR Cache to be used.
+     *                 If {@code null}, value of {@link JarCache#getDefault()} will be used.
+     * @return {@code this}
+     * @since 3.12
+     * @throws IOException Default JAR Cache location cannot be initialized
+     */
+    public ChannelBuilder withJarCacheOrDefault(@CheckForNull JarCache jarCache) throws IOException {
+        this.jarCache = jarCache != null ? jarCache : JarCache.getDefault();
+        return this;
+    }
+
+    /**
+     * Resets JAR Cache setting to the default.
+     * The behavior will be determined by the {@link Channel} implementation.
+     *
+     * @since 3.12
+     */
+    public ChannelBuilder withoutJarCache() {
+        this.jarCache = null;
+        return this;
+    }
+
     /**
      * Gets the JAR Cache storage.
      * @return {@code null} if it is not defined.
-     *         {@link Channel} implementation should use a default cache value then.
+     *         {@link Channel} implementation defines the behavior in such case.
+     * @since 2.38
      */
     @CheckForNull
     public JarCache getJarCache() {

diff --git a/src/main/java/hudson/remoting/Engine.java b/src/main/java/hudson/remoting/Engine.java
@@ -263,7 +263,11 @@ public synchronized void startEngine() throws IOException {
                 throw new IOException("Cannot find the JAR Cache location");
             }
             LOGGER.log(Level.FINE, "Using standard File System JAR Cache. Root Directory is {0}", jarCacheDirectory);
-            jarCache = new FileSystemJarCache(jarCacheDirectory, true);
+            try {
+                jarCache = new FileSystemJarCache(jarCacheDirectory, true);
+            } catch (IllegalArgumentException ex) {
+                throw new IOException("Failed to initialize FileSystem JAR Cache in " + jarCacheDirectory, ex);
+            }
         } else {
             LOGGER.log(Level.INFO, "Using custom JAR Cache: {0}", jarCache);
         }
@@ -567,7 +571,10 @@ public void afterProperties(@Nonnull JnlpConnectionState event) {
 
                                 @Override
                                 public void beforeChannel(@Nonnull JnlpConnectionState event) {
-                                    event.getChannelBuilder().withJarCache(jarCache).withMode(Mode.BINARY);
+                                    ChannelBuilder bldr = event.getChannelBuilder().withMode(Mode.BINARY);
+                                    if (jarCache != null) {
+                                        bldr.withJarCache(jarCache);
+                                    }
                                 }
 
                                 @Override

diff --git a/src/main/java/hudson/remoting/FileSystemJarCache.java b/src/main/java/hudson/remoting/FileSystemJarCache.java
@@ -36,6 +36,7 @@ public class FileSystemJarCache extends JarCacheSupport {
     @GuardedBy("itself")
     private final Map<String, Checksum> checksumsByPath = new HashMap<>();
 
+    //TODO: Create new IOException constructor
     /**
      * @param rootDir  
      *      Root directory.
@@ -54,7 +55,7 @@ public FileSystemJarCache(@Nonnull File rootDir, boolean touch) {
         try {
             Util.mkdirs(rootDir);
         } catch (IOException ex) {
-            throw new RuntimeException("Root directory not writable: " + rootDir);
+            throw new IllegalArgumentException("Root directory not writable: " + rootDir);
         }
     }
 

diff --git a/src/main/java/hudson/remoting/JarCache.java b/src/main/java/hudson/remoting/JarCache.java
@@ -26,10 +26,20 @@ public abstract class JarCache {
      */
     /*package*/ static final File DEFAULT_NOWS_JAR_CACHE_LOCATION = 
         new File(System.getProperty("user.home"),".jenkins/cache/jars");
-
+
+    //TODO: replace by checked exception
+    /**
+     * Gets a default value for {@link FileSystemJarCache} to be initialized on agents.
+     * @return Created JAR Cache
+     * @throws IOException Default JAR Cache location cannot be initialized
+     */
     @Nonnull
-    /*package*/ static JarCache getDefault() {
-        return new FileSystemJarCache(DEFAULT_NOWS_JAR_CACHE_LOCATION, true);
+    /*package*/ static JarCache getDefault() throws IOException {
+        try {
+            return new FileSystemJarCache(DEFAULT_NOWS_JAR_CACHE_LOCATION, true);
+        } catch (IllegalArgumentException ex) {
+            throw new IOException("Failed to initialize the default JAR Cache location", ex);
+        }
     }
 
     /**

diff --git a/src/main/java/hudson/remoting/Launcher.java b/src/main/java/hudson/remoting/Launcher.java
@@ -732,7 +732,7 @@ public static void main(InputStream is, OutputStream os, Mode mode, boolean perf
         ExecutorService executor = Executors.newCachedThreadPool();
         ChannelBuilder cb = new ChannelBuilder("channel", executor)
                 .withMode(mode)
-                .withJarCache(cache);
+                .withJarCacheOrDefault(cache);
 
         // expose StandardOutputStream as a channel property, which is a better way to make this available
         // to the user of Channel than Channel#getUnderlyingOutput()

diff --git a/src/main/java/hudson/remoting/ResourceImageBoth.java b/src/main/java/hudson/remoting/ResourceImageBoth.java
@@ -42,7 +42,9 @@ Future<URLish> resolveURL(Channel channel, String resourcePath) throws IOExcepti
     @Nonnull
     private Future<URL> initiateJarRetrieval(@Nonnull Channel channel) throws IOException, InterruptedException {
         JarCache c = channel.getJarCache();
-        assert c !=null : "we don't advertise jar caching to the other side unless we have a cache with us";
+        if (c == null) {
+            throw new IOException("Failed to initiate retrieval. JAR Cache is disabled for the channel " + channel.getName());
+        }
 
         try {
             return c.resolve(channel, sum1, sum2);

diff --git a/src/main/java/hudson/remoting/ResourceImageInJar.java b/src/main/java/hudson/remoting/ResourceImageInJar.java
@@ -82,7 +82,10 @@ open jar file (see sun.net.www.protocol.jar.JarURLConnection.JarURLInputStream.c
 
     Future<URL> _resolveJarURL(Channel channel) throws IOException, InterruptedException {
         JarCache c = channel.getJarCache();
-        assert c !=null : "we don't advertise jar caching to the other side unless we have a cache with us";
+        if (c == null) {
+            throw new IOException(String.format("Failed to resolve a jar %016x%016x. JAR Cache is disabled for the channel %s",
+                    sum1, sum2, channel.getName()));
+        }
 
         return c.resolve(channel, sum1, sum2);
 //            throw (IOException)new IOException(String.format("Failed to resolve a jar %016x%016x",sum1,sum2)).initCause(e);

diff --git a/src/main/java/org/jenkinsci/remoting/nio/NioChannelBuilder.java b/src/main/java/org/jenkinsci/remoting/nio/NioChannelBuilder.java
@@ -84,6 +84,11 @@ public NioChannelBuilder withJarCache(JarCache jarCache) {
         return (NioChannelBuilder) super.withJarCache(jarCache);
     }
 
+    @Override
+    public NioChannelBuilder withoutJarCache() {
+        return (NioChannelBuilder) super.withoutJarCache();
+    }
+
     @Override
     public NioChannelBuilder withClassFilter(ClassFilter filter) {
         return (NioChannelBuilder)super.withClassFilter(filter);

diff --git a/src/test/java/hudson/remoting/PrefetchingTest.java b/src/test/java/hudson/remoting/PrefetchingTest.java
@@ -215,7 +215,11 @@ private ForceJarLoad(Checksum sum) {
         public Void call() throws IOException {
             try {
                 Channel ch = Channel.current();
-                ch.getJarCache().resolve(ch,sum1,sum2).get();
+                final JarCache jarCache = ch.getJarCache();
+                if (jarCache == null) {
+                    throw new IOException("Cannot Force JAR load, JAR cache is disabled");
+                }
+                jarCache.resolve(ch,sum1,sum2).get();
                 return null;
             } catch (InterruptedException e) {
                 throw new IOException(e);