[JENKINS-44973] - Ensure that JAR Cache management works correctly in…

… Launcher It is an unreleased regression caused by me in #166 Discovered it during testing the recent remoting version on SSH. Likely some Unit tests are also unstable due to it.
jenkinsci · Jun 19, 2017 · 589275e · Vlatombe · Sep 12, 2017 · 589275e
1 parent 638dbde
commit 589275e
Show file tree

Hide file tree

Showing 7 changed files with 54 additions and 19 deletions.
diff --git a/src/main/java/hudson/remoting/Channel.java b/src/main/java/hudson/remoting/Channel.java
@@ -317,6 +317,7 @@ public class Channel implements VirtualChannel, IChannel, Closeable {
      */
     /*package*/ final ClassLoader baseClassLoader;
 
+    @Nonnull
     private JarCache jarCache;
 
     /*package*/ final JarLoaderImpl jarLoader;
@@ -490,14 +491,21 @@ public Channel(String name, ExecutorService exec, CommandTransport transport, bo
     /**
      * @since TODO
      */
-    protected Channel(ChannelBuilder settings, CommandTransport transport) throws IOException {
+    protected Channel(@Nonnull ChannelBuilder settings, @Nonnull CommandTransport transport) throws IOException {
         this.name = settings.getName();
         this.reference = new Ref(this);
         this.executor = new InterceptingExecutorService(settings.getExecutors(),decorators);
         this.arbitraryCallableAllowed = settings.isArbitraryCallableAllowed();
         this.remoteClassLoadingAllowed = settings.isRemoteClassLoadingAllowed();
         this.underlyingOutput = transport.getUnderlyingStream();
-        this.jarCache = settings.getJarCache();
+
+        // 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 = effectiveJarCache;
 
         this.baseClassLoader = settings.getBaseLoader();
         this.classFilter = settings.getClassFilter();
@@ -781,6 +789,7 @@ 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.
      * @since 2.24
      */
+    @Nonnull
     public JarCache getJarCache() {
         return jarCache;
     }
@@ -793,7 +802,7 @@ public JarCache getJarCache() {
      * unless your call sequence guarantees that you call this method before remote classes are loaded.
      * @since 2.24
      */
-    public void setJarCache(JarCache jarCache) {
+    public void setJarCache(@Nonnull JarCache jarCache) {
         this.jarCache = jarCache;
     }
 

diff --git a/src/main/java/hudson/remoting/ChannelBuilder.java b/src/main/java/hudson/remoting/ChannelBuilder.java
@@ -26,6 +26,7 @@
 import java.util.Map;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
+import javax.annotation.CheckForNull;
 
 /**
  * Factory for {@link Channel}, including hand-shaking between two sides
@@ -46,6 +47,7 @@ public class ChannelBuilder {
     private Mode mode = Mode.NEGOTIATE;
     private Capability capability = new Capability();
     private OutputStream header;
+    @CheckForNull
     private JarCache jarCache;
     private List<CallableDecorator> decorators = new ArrayList<CallableDecorator>();
     private boolean arbitraryCallableAllowed = true;
@@ -188,11 +190,22 @@ public boolean isRemoteClassLoadingAllowed() {
         return remoteClassLoadingAllowed;
     }
 
-    public ChannelBuilder withJarCache(JarCache jarCache) {
+    /**
+     * 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}
+     * @return {@code this}
+     */
+    public ChannelBuilder withJarCache(@CheckForNull JarCache jarCache) {
         this.jarCache = jarCache;
         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.
+     */
+    @CheckForNull
     public JarCache getJarCache() {
         return jarCache;
     }

diff --git a/src/main/java/hudson/remoting/Engine.java b/src/main/java/hudson/remoting/Engine.java
@@ -155,11 +155,7 @@ public void run() {
     private boolean keepAlive = true;
 
 
-    /**
-     * Default JAR cache location for disabled workspace Manager.
-     */
-    /*package*/ static final File DEFAULT_NOWS_JAR_CACHE_LOCATION = 
-        new File(System.getProperty("user.home"),".jenkins/cache/jars");
+
 
     @CheckForNull
     private JarCache jarCache = null;
@@ -257,8 +253,8 @@ public synchronized void startEngine() throws IOException {
             jarCacheDirectory = workDirManager.getLocation(WorkDirManager.DirType.JAR_CACHE_DIR);
             workDirManager.setupLogging(path, agentLog);
         } else if (jarCache == null) {
-            LOGGER.log(Level.WARNING, "No Working Directory. Using the legacy JAR Cache location: {0}", DEFAULT_NOWS_JAR_CACHE_LOCATION);
-            jarCacheDirectory = DEFAULT_NOWS_JAR_CACHE_LOCATION;
+            LOGGER.log(Level.WARNING, "No Working Directory. Using the legacy JAR Cache location: {0}", JarCache.DEFAULT_NOWS_JAR_CACHE_LOCATION);
+            jarCacheDirectory = JarCache.DEFAULT_NOWS_JAR_CACHE_LOCATION;
         }
 
         if (jarCache == null){

diff --git a/src/main/java/hudson/remoting/FileSystemJarCache.java b/src/main/java/hudson/remoting/FileSystemJarCache.java
@@ -49,7 +49,7 @@ public FileSystemJarCache(@Nonnull File rootDir, boolean touch) {
         this.rootDir = rootDir;
         this.touch = touch;
         if (rootDir==null)
-            throw new IllegalArgumentException();
+            throw new IllegalArgumentException("Root directory is null");
 
         try {
             Util.mkdirs(rootDir);

diff --git a/src/main/java/hudson/remoting/JarCache.java b/src/main/java/hudson/remoting/JarCache.java
@@ -1,8 +1,10 @@
 package hudson.remoting;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.net.URL;
+import javax.annotation.Nonnull;
 
 /**
  * Jar file cache.
@@ -18,6 +20,18 @@
  * @since 2.24
  */
 public abstract class JarCache {
+
+    /**
+     * Default JAR cache location for disabled workspace Manager.
+     */
+    /*package*/ static final File DEFAULT_NOWS_JAR_CACHE_LOCATION = 
+        new File(System.getProperty("user.home"),".jenkins/cache/jars");
+
+    @Nonnull
+    /*package*/ static JarCache getDefault() {
+        return new FileSystemJarCache(DEFAULT_NOWS_JAR_CACHE_LOCATION, true);
+    }
+
     /**
      * Looks up the jar in cache, and if not found, use {@link JarLoader} to retrieve it
      * from the other side.

diff --git a/src/main/java/hudson/remoting/Launcher.java b/src/main/java/hudson/remoting/Launcher.java
@@ -173,6 +173,7 @@ public void addClasspath(String pathList) throws Exception {
     /**
      * @since 2.24
      */
+    @CheckForNull
     @Option(name="-jar-cache",metaVar="DIR",usage="Cache directory that stores jar files sent from the master")
     public File jarCache = null;
 
@@ -622,7 +623,7 @@ private void runOnSocket(Socket s) throws IOException, InterruptedException {
         s.setTcpNoDelay(true);
         main(new BufferedInputStream(SocketChannelStream.in(s)),
              new BufferedOutputStream(SocketChannelStream.out(s)), mode,ping,
-             new FileSystemJarCache(jarCache,true));
+             jarCache != null ? new FileSystemJarCache(jarCache,true) : null);
     }
 
     /**
@@ -676,7 +677,7 @@ private void runWithStdinStdout() throws IOException, InterruptedException {
 
         // System.in/out appear to be already buffered (at least that was the case in Linux and Windows as of Java6)
         // so we are not going to double-buffer these.
-        main(System.in, os, mode, ping, new FileSystemJarCache(jarCache,true));
+        main(System.in, os, mode, ping, jarCache != null ? new FileSystemJarCache(jarCache,true) : null);
     }
 
     private static void ttyCheck() {
@@ -719,13 +720,15 @@ public static void main(InputStream is, OutputStream os, Mode mode) throws IOExc
      */
     @Deprecated
     public static void main(InputStream is, OutputStream os, Mode mode, boolean performPing) throws IOException, InterruptedException {
-        main(is, os, mode, performPing,
-                new FileSystemJarCache(new File(System.getProperty("user.home"),".jenkins/cache/jars"),true));
+        main(is, os, mode, performPing, null);
     }
     /**
+     * 
+     * @param cache JAR cache to be used.
+     *              If {@code null}, a default value will be used.
      * @since 2.24
      */
-    public static void main(InputStream is, OutputStream os, Mode mode, boolean performPing, JarCache cache) throws IOException, InterruptedException {
+    public static void main(InputStream is, OutputStream os, Mode mode, boolean performPing, @CheckForNull JarCache cache) throws IOException, InterruptedException {
         ExecutorService executor = Executors.newCachedThreadPool();
         ChannelBuilder cb = new ChannelBuilder("channel", executor)
                 .withMode(mode)

diff --git a/src/test/java/hudson/remoting/EngineTest.java b/src/test/java/hudson/remoting/EngineTest.java
@@ -66,8 +66,8 @@ public void shouldInitializeCorrectlyWithDefaults() throws Exception {
         engine.startEngine(true);
 
         // Cache will go to ~/.jenkins , we do not want to worry anbout this repo
-        Assert.assertTrue("Default JarCache should be touched: " + Engine.DEFAULT_NOWS_JAR_CACHE_LOCATION.getAbsolutePath(), 
-                Engine.DEFAULT_NOWS_JAR_CACHE_LOCATION.exists());
+        Assert.assertTrue("Default JarCache should be touched: " + JarCache.DEFAULT_NOWS_JAR_CACHE_LOCATION.getAbsolutePath(), 
+                JarCache.DEFAULT_NOWS_JAR_CACHE_LOCATION.exists());
     }
 
     @Test