Merge pull request #96 from jglick/saveProgramIfPossible-JENKINS-29656

[JENKINS-29656] Better handle failure renaming program.dat
jenkinsci · Jan 17, 2017 · 89ce0aa · 89ce0aa
2 parents 12934f4 + 20c6a30
commit 89ce0aa
Showing 1 changed file with 21 additions and 8 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java
@@ -28,6 +28,7 @@
 import com.cloudbees.groovy.cps.Outcome;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.SettableFuture;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import groovy.lang.Closure;
 import groovy.lang.GroovyShell;
 import groovy.lang.Script;
@@ -225,13 +226,13 @@ public Future<?> scheduleRun() {
         final SettableFuture<Void> f = SettableFuture.create();
         try {
             runner.submit(new Callable<Void>() {
-                @edu.umd.cs.findbugs.annotations.SuppressWarnings("RV_RETURN_VALUE_IGNORED_BAD_PRACTICE") // runner.submit() result
+                @SuppressFBWarnings(value="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE", justification="runner.submit() result")
                 public Void call() throws Exception {
                     Jenkins j = Jenkins.getInstance();
                     if (paused.get() || j == null || j.isQuietingDown()) {
                         // by doing the pause check inside, we make sure that scheduleRun() returns a
                         // future that waits for any previously scheduled tasks to be completed.
-                        saveProgram();
+                        saveProgramIfPossible();
                         f.set(null);
                         return null;
                     }
@@ -356,11 +357,7 @@ private boolean run() {
         }
 
         if (changed && !stillRunnable) {
-            try {
-                saveProgram();
-            } catch (IOException x) {
-                LOGGER.log(WARNING, "program state save failed", x);
-            }
+            saveProgramIfPossible();
         }
         if (ending) {
             execution.cleanUpHeap();
@@ -409,6 +406,18 @@ public CpsThreadDump getThreadDump() {
         return CpsThreadDump.from(this);
     }
 
+    /**
+     * Like {@link #saveProgram()} but will not fail.
+     */
+    @CpsVmThreadOnly
+    void saveProgramIfPossible() {
+        try {
+            saveProgram();
+        } catch (IOException x) {
+            LOGGER.log(WARNING, "program state save failed", x);
+        }
+    }
+
     /**
      * Persists the current state of {@link CpsThreadGroup}.
      */
@@ -436,20 +445,24 @@ public void saveProgram(File f) throws IOException {
             return;
         }
 
+        boolean serializedOK = false;
         try {
             RiverWriter w = new RiverWriter(tmpFile, execution.getOwner());
             try {
                 w.writeObject(this);
             } finally {
                 w.close();
             }
+            serializedOK = true;
             Files.move(tmpFile.toPath(), f.toPath(), StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
             LOGGER.log(FINE, "program state saved");
         } catch (RuntimeException e) {
             propagateErrorToWorkflow(e);
             throw new IOException("Failed to persist "+f,e);
         } catch (IOException e) {
-            propagateErrorToWorkflow(e);
+            if (!serializedOK) {
+                propagateErrorToWorkflow(e);
+            } // JENKINS-29656: otherwise just send the I/O error to caller and move on
             throw new IOException("Failed to persist "+f,e);
         } finally {
             PROGRAM_STATE_SERIALIZATION.set(old);