[HUDSON-5073] Hudson was failing to record the connection termination problem in slave logs.

This change might reveal additional information about why the connection is disrupted.

changelog.html

@@ -46,6 +46,8 @@ Upcoming changes</a>
   <li class=bug>
     M2 and M3 builds behave differently when tests fail.
     (<a href="http://issues.hudson-ci.org/browse/HUDSON-8415">issue 8415</a>)
+  <li class=bug>
+    Hudson was failing to record the connection termination problem in slave logs.
   <li class=rfe>
     Startup performance improvement
   <li class=rfe>

core/src/main/java/hudson/slaves/CommandLauncher.java

@@ -120,10 +120,15 @@ public class CommandLauncher extends ComputerLauncher {
             computer.setChannel(proc.getInputStream(), proc.getOutputStream(), listener.getLogger(), new Channel.Listener() {
                 @Override
                 public void onClosed(Channel channel, IOException cause) {
-                    if (cause != null) {
-                        cause.printStackTrace(
-                            listener.error(hudson.model.Messages.Slave_Terminated(getTimestamp())));
+                    try {
+                        int exitCode = proc.exitValue();
+                        if (exitCode!=0) {
+                            listener.error("Process terminated with exit code "+exitCode);
+                        }
+                    } catch (IllegalThreadStateException e) {
+                        // hasn't terminated yet
                     }
+
                     try {
                         ProcessTree.get().killAll(proc, cookie);
                     } catch (InterruptedException e) {

core/src/main/java/hudson/slaves/ComputerLauncher.java

@@ -63,7 +63,7 @@ public abstract class ComputerLauncher extends AbstractDescribableImpl<ComputerL
      * Launches the slave agent for the given {@link Computer}.
      *
      * <p>
-     * If the slave agent is launched successfully, {@link SlaveComputer#setChannel(InputStream, OutputStream, TaskListener, Listener)}
+     * If the slave agent is launched successfully, {@link SlaveComputer#setChannel(InputStream, OutputStream, TaskListener, Channel.Listener)}
      * should be invoked in the end to notify Hudson of the established connection.
      * The operation could also fail, in which case there's no need to make any callback notification,
      * (except to notify the user of the failure through {@link StreamTaskListener}.)

core/src/main/java/hudson/slaves/SlaveComputer.java

@@ -58,6 +58,7 @@ import java.nio.charset.Charset;
 import java.util.concurrent.Future;
 import java.security.Security;
 
+import hudson.util.io.ReopenableFileOutputStream;
 import org.apache.commons.io.IOUtils;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
@@ -88,6 +89,17 @@ public class SlaveComputer extends Computer {
      */
     private ComputerLauncher launcher;
 
+    /**
+     * Perpetually writable log file.
+     */
+    private final ReopenableFileOutputStream log;
+
+    /**
+     * {@link StreamTaskListener} that wraps {@link #log}, hence perpetually writable.
+     */
+    private final TaskListener taskListener;
+
+
     /**
      * Number of failed attempts to reconnect to this node
      * (so that if we keep failing to reconnect, we can stop
@@ -105,6 +117,8 @@ public class SlaveComputer extends Computer {
 
     public SlaveComputer(Slave slave) {
         super(slave);
+        this.log = new ReopenableFileOutputStream(getLogFile());
+        this.taskListener = new StreamTaskListener(log);
     }
 
     /**
@@ -177,25 +191,23 @@ public class SlaveComputer extends Computer {
             public Object call() throws Exception {
                 // do this on another thread so that the lengthy launch operation
                 // (which is typical) won't block UI thread.
-                OutputStream out = openLogFile();
                 try {
-                    TaskListener listener = new StreamTaskListener(out);
+                    log.rewind();
                     try {
-                        launcher.launch(SlaveComputer.this, listener);
+                        launcher.launch(SlaveComputer.this, taskListener);
                         return null;
                     } catch (AbortException e) {
-                        listener.error(e.getMessage());
+                        taskListener.error(e.getMessage());
                         throw e;
                     } catch (IOException e) {
-                        Util.displayIOException(e,listener);
-                        e.printStackTrace(listener.error(Messages.ComputerLauncher_unexpectedError()));
+                        Util.displayIOException(e,taskListener);
+                        e.printStackTrace(taskListener.error(Messages.ComputerLauncher_unexpectedError()));
                         throw e;
                     } catch (InterruptedException e) {
-                        e.printStackTrace(listener.error(Messages.ComputerLauncher_abortedLaunch()));
+                        e.printStackTrace(taskListener.error(Messages.ComputerLauncher_abortedLaunch()));
                         throw e;
                     }
                 } finally {
-                    IOUtils.closeQuietly(out);
                     if (channel==null)
                         offlineCause = new OfflineCause.LaunchFailed();
                 }
@@ -254,14 +266,13 @@ public class SlaveComputer extends Computer {
     }
 
     public OutputStream openLogFile() {
-        OutputStream os;
         try {
-            os = new FileOutputStream(getLogFile());
-        } catch (FileNotFoundException e) {
+            log.rewind();
+            return log;
+        } catch (IOException e) {
             logger.log(Level.SEVERE, "Failed to create log file "+getLogFile(),e);
-            os = new NullStream();
+            return new NullStream();
         }
-        return os;
     }
 
     private final Object channelLock = new Object();
@@ -286,6 +297,8 @@ public class SlaveComputer extends Computer {
      *      be useful for debugging/trouble-shooting.
      * @param listener
      *      Gets a notification when the channel closes, to perform clean up. Can be null.
+     *      By the time this method is called, the cause of the termination is reported to the user,
+     *      so the implementation of the listener doesn't need to do that again.
      */
     public void setChannel(InputStream in, OutputStream out, OutputStream launchLog, Channel.Listener listener) throws IOException, InterruptedException {
         if(this.channel!=null)
@@ -301,7 +314,12 @@ public class SlaveComputer extends Computer {
             public void onClosed(Channel c, IOException cause) {
                 SlaveComputer.this.channel = null;
                 // Orderly shutdown will have null exception
-                if (cause!=null) offlineCause = new ChannelTermination(cause);
+                if (cause!=null) {
+                    offlineCause = new ChannelTermination(cause);
+                     cause.printStackTrace(taskListener.error("Connection terminated"));
+                } else {
+                    taskListener.getLogger().println("Connection terminated");
+                }
                 launcher.afterDisconnect(SlaveComputer.this, taskListener);
             }
         });
@@ -391,15 +409,9 @@ public class SlaveComputer extends Computer {
             public void run() {
                 // do this on another thread so that any lengthy disconnect operation
                 // (which could be typical) won't block UI thread.
-                OutputStream out = openLogFile();
-                try {
-                    TaskListener listener = new StreamTaskListener(out);
-                    launcher.beforeDisconnect(SlaveComputer.this, listener);
-                    closeChannel();
-                    launcher.afterDisconnect(SlaveComputer.this, listener);
-                } finally {
-                    IOUtils.closeQuietly(out);
-                }
+                launcher.beforeDisconnect(SlaveComputer.this, taskListener);
+                closeChannel();
+                launcher.afterDisconnect(SlaveComputer.this, taskListener);
             }
         });
     }

core/src/main/java/hudson/util/io/ReopenableFileOutputStream.java

+/*
+ * The MIT License
+ *
+ * Copyright (c) 2010, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package hudson.util.io;
+
+import hudson.util.IOException2;
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+
+/**
+ * {@link OutputStream} that writes to a file.
+ *
+ * <p>
+ * Unlike regular {@link FileOutputStream}, this implementation allows the caller to close,
+ * and then keep writing.
+ *
+ * @author Kohsuke Kawaguchi
+ */
+public class ReopenableFileOutputStream extends OutputStream {
+    private final File out;
+
+    private OutputStream current;
+    private boolean appendOnNextOpen = false;
+
+    public ReopenableFileOutputStream(File out) {
+        this.out = out;
+    }
+
+    private synchronized OutputStream current() throws IOException {
+        if (current==null)
+            try {
+                current = new FileOutputStream(out,appendOnNextOpen);
+            } catch (FileNotFoundException e) {
+                throw new IOException2("Failed to open "+out,e);
+            }
+        return current;
+    }
+
+    @Override
+    public void write(int b) throws IOException {
+        current().write(b);
+    }
+
+    @Override
+    public void write(byte[] b) throws IOException {
+        current().write(b);
+    }
+
+    @Override
+    public void write(byte[] b, int off, int len) throws IOException {
+        current().write(b, off, len);
+    }
+
+    @Override
+    public void flush() throws IOException {
+        current().flush();
+    }
+
+    @Override
+    public synchronized void close() throws IOException {
+        if (current!=null) {
+            current.close();
+            appendOnNextOpen = true;
+            current = null;
+        }
+    }
+
+    /**
+     * In addition to close, ensure that the next "open" would truncate the file.
+     */
+    public synchronized void rewind() throws IOException {
+        close();
+        appendOnNextOpen = false;
+    }
+}