From 07db9c85048d5072b9adc602c6cf05091b8aafd7 Mon Sep 17 00:00:00 2001
From: Kohsuke Kawaguchi <kk@kohsuke.org>
Date: Tue, 1 Apr 2014 11:31:33 -0700
Subject: [PATCH] error handling improvement and bug fix

---
 .../JnlpSlaveRestarterInstaller.java          | 77 +++++++++++--------
 1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/core/src/main/java/jenkins/slaves/restarter/JnlpSlaveRestarterInstaller.java b/core/src/main/java/jenkins/slaves/restarter/JnlpSlaveRestarterInstaller.java
index 97462b9d24..73b3b4df9e 100644
--- a/core/src/main/java/jenkins/slaves/restarter/JnlpSlaveRestarterInstaller.java
+++ b/core/src/main/java/jenkins/slaves/restarter/JnlpSlaveRestarterInstaller.java
@@ -11,6 +11,7 @@ import hudson.remoting.VirtualChannel;
 import hudson.slaves.ComputerListener;
 
 import java.io.IOException;
+import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
@@ -28,7 +29,7 @@ import static java.util.logging.Level.*;
  * @author Kohsuke Kawaguchi
  */
 @Extension
-public class JnlpSlaveRestarterInstaller extends ComputerListener {
+public class JnlpSlaveRestarterInstaller extends ComputerListener implements Serializable {
     @Override
     public void onOnline(Computer c, TaskListener listener) throws IOException, InterruptedException {
         final List<SlaveRestarter> restarters = new ArrayList<SlaveRestarter>(SlaveRestarter.all());
@@ -36,50 +37,58 @@ public class JnlpSlaveRestarterInstaller extends ComputerListener {
         VirtualChannel ch = c.getChannel();
         if (ch==null)   return; // defensive check
 
-        List<SlaveRestarter> effective = ch.call(new Callable<List<SlaveRestarter>, IOException>() {
-            public List<SlaveRestarter> call() throws IOException {
-                Engine e = Engine.current();
-                if (e == null) return null;    // not running under Engine
+        List<SlaveRestarter> effective = null;
+        try {
+            effective = ch.call(new Callable<List<SlaveRestarter>, IOException>() {
+                public List<SlaveRestarter> call() throws IOException {
+                    Engine e = Engine.current();
+                    if (e == null) return null;    // not running under Engine
 
-                try {
-                    Engine.class.getMethod("addListener", EngineListener.class);
-                } catch (NoSuchMethodException _) {
-                    return null;    // running with older version of remoting that doesn't support adding listener
-                }
+                    try {
+                        Engine.class.getMethod("addListener", EngineListener.class);
+                    } catch (NoSuchMethodException _) {
+                        return null;    // running with older version of remoting that doesn't support adding listener
+                    }
 
-                // filter out ones that doesn't apply
-                for (Iterator<SlaveRestarter> itr = restarters.iterator(); itr.hasNext(); ) {
-                    SlaveRestarter r =  itr.next();
-                    if (!r.canWork())
-                        itr.remove();
-                }
+                    // filter out ones that doesn't apply
+                    for (Iterator<SlaveRestarter> itr = restarters.iterator(); itr.hasNext(); ) {
+                        SlaveRestarter r =  itr.next();
+                        if (!r.canWork())
+                            itr.remove();
+                    }
 
-                e.addListener(new EngineListenerAdapter() {
-                    @Override
-                    public void onDisconnect() {
-                        try {
-                            for (SlaveRestarter r : restarters) {
-                                try {
-                                    r.restart();
-                                } catch (Exception x) {
-                                    LOGGER.log(SEVERE, "Failed to restart slave with "+r, x);
+                    e.addListener(new EngineListenerAdapter() {
+                        @Override
+                        public void onDisconnect() {
+                            try {
+                                for (SlaveRestarter r : restarters) {
+                                    try {
+                                        r.restart();
+                                    } catch (Exception x) {
+                                        LOGGER.log(SEVERE, "Failed to restart slave with "+r, x);
+                                    }
                                 }
+                            } finally {
+                                // if we move on to the reconnection without restart,
+                                // don't let the current implementations kick in when the slave loses connection again
+                                restarters.clear();
                             }
-                        } finally {
-                            // if we move on to the reconnection without restart,
-                            // don't let the current implementations kick in when the slave loses connection again
-                            restarters.clear();
                         }
-                    }
-                });
+                    });
 
-                return restarters;
-            }
-        });
+                    return restarters;
+                }
+            });
+        } catch (IOException e) {
+            e.printStackTrace(listener.error("Failed to install restarter"));
+            // don't let this fail the slave connection
+        }
 
         // TODO: report this to GUI
         LOGGER.fine("Effective SlaveRestarter on "+c.getDisplayName()+": "+effective);
     }
 
     private static final Logger LOGGER = Logger.getLogger(JnlpSlaveRestarterInstaller.class.getName());
+
+    private static final long serialVersionUID = 1L;
 }
-- 
GitLab