[FIXED JENKINS-29568] A better fix

- 252e1296 doesn't work for all cases as there are code paths
  where a pending launch can be removed from the pending list and not have spent() called.
- There was no reason for iterating the list twice anyway, as all of this takes place with the locks held
- My notifying each one as we process, if there is an Error, we will not leave any stranded. The next run
  through, if there is one, will cover those instances.

(cherry picked from commit 4f0ca16b)

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

@@ -213,48 +213,50 @@ public class NodeProvisioner {
                     // bring up.
 
                     int plannedCapacitySnapshot = 0;
-                    List<PlannedNode> completedLaunches = new ArrayList<PlannedNode>();
 
                     for (Iterator<PlannedNode> itr = pendingLaunches.iterator(); itr.hasNext(); ) {
                         PlannedNode f = itr.next();
                         if (f.future.isDone()) {
-                            completedLaunches.add(f);
-                            itr.remove();
+                            try {
+                                Node node = f.future.get();
+                                for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
+                                    cl.onComplete(f, node);
+                                }
+    
+                                jenkins.addNode(node);
+                                LOGGER.log(Level.INFO,
+                                        "{0} provisioning successfully completed. " 
+                                                + "We have now {1,number,integer} computer(s)",
+                                        new Object[]{f.displayName, jenkins.getComputers().length});
+                            } catch (InterruptedException e) {
+                                throw new AssertionError(e); // since we confirmed that the future is already done
+                            } catch (ExecutionException e) {
+                                LOGGER.log(Level.WARNING, "Provisioned slave " + f.displayName + " failed to launch",
+                                        e.getCause());
+                                for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
+                                    cl.onFailure(f, e.getCause());
+                                }
+                            } catch (IOException e) {
+                                LOGGER.log(Level.WARNING, "Provisioned slave " + f.displayName + " failed to launch", 
+                                        e);
+                                for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
+                                    cl.onFailure(f, e);
+                                }
+                            } catch (Error e) {
+                                // we are not supposed to try and recover from Errors
+                                throw e;
+                            } catch (Throwable e) {
+                                LOGGER.log(Level.SEVERE, "Unexpected uncaught exception encountered while " 
+                                        + "processing provisioned slave " + f.displayName, e);
+                            } finally {
+                                itr.remove();
+                                f.spent();
+                            }
                         } else {
                             plannedCapacitySnapshot += f.numExecutors;
                         }
                     }
 
-                    for (PlannedNode f : completedLaunches) {
-                        try {
-                            Node node = f.future.get();
-                            for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
-                                cl.onComplete(f, node);
-                            }
-
-                            jenkins.addNode(node);
-                            LOGGER.log(Level.INFO,
-                                    "{0} provisioning successfully completed. We have now {1,number,integer} computer"
-                                            + "(s)",
-                                    new Object[]{f.displayName, jenkins.getComputers().length});
-                        } catch (InterruptedException e) {
-                            throw new AssertionError(e); // since we confirmed that the future is already done
-                        } catch (ExecutionException e) {
-                            LOGGER.log(Level.WARNING, "Provisioned slave " + f.displayName + " failed to launch",
-                                    e.getCause());
-                            for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
-                                cl.onFailure(f, e.getCause());
-                            }
-                        } catch (IOException e) {
-                            LOGGER.log(Level.WARNING, "Provisioned slave " + f.displayName + " failed to launch", e);
-                            for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
-                                cl.onFailure(f, e);
-                            }
-                        } finally {
-                            f.spent();
-                        }
-                    }
-
                     float plannedCapacity = plannedCapacitySnapshot;
                     plannedCapacitiesEMA.update(plannedCapacity);