[FIXED JENKINS-19515] Try to avoid truncation of fingerprint storage …

…files. 1. Use AtomicFileWriter on the hypothesis that an exception during save() caused truncation. 2. Improve logging from the cleanup thread since that may be related to the root cause. 3. Regardless of the cause, recover more gracefully by deleting any truncated file rather than throwing the error up.
jenkinsci · Sep 9, 2013 · 7790dbc · 7790dbc
1 parent 89a1146
commit 7790dbc
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 7 deletions.
diff --git a/changelog.html b/changelog.html
@@ -67,6 +67,9 @@
   <li class=bug>
     Since 1.518, fingerprint serialization broke when job or file names contained XML special characters like ampersands.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-18337">issue 18337</a>)
+  <li class=bug>
+    Robustness against truncated fingerprint files.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-19515">issue 19515</a>)
   <li class=rfe>
     JavaScript error in the checkUrl computation shouldn't break the job configuration page.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-19457">issue 19457</a>)

diff --git a/core/src/main/java/hudson/model/Fingerprint.java b/core/src/main/java/hudson/model/Fingerprint.java
@@ -38,11 +38,13 @@
 import hudson.Extension;
 import hudson.model.listeners.ItemListener;
 import hudson.model.listeners.SaveableListener;
+import hudson.util.AtomicFileWriter;
 import hudson.util.HexBinaryConverter;
 import hudson.util.Iterators;
 import hudson.util.PersistedList;
 import hudson.util.RunList;
 import hudson.util.XStream2;
+import java.io.EOFException;
 import jenkins.model.FingerprintFacet;
 import jenkins.model.Jenkins;
 import jenkins.model.TransientFingerprintFacetFactory;
@@ -995,8 +997,12 @@ public synchronized boolean trim() throws IOException {
             }
         }
 
-        if (modified)
+        if (modified) {
+            if (logger.isLoggable(Level.FINE)) {
+                logger.log(Level.FINE, "Saving trimmed {0}", getFingerprintFile(md5sum));
+            }
             save();
+        }
 
         return modified;
     }
@@ -1101,8 +1107,9 @@ void save(File file) throws IOException {
         if (facets.isEmpty()) {
             file.getParentFile().mkdirs();
             // JENKINS-16301: fast path for the common case.
-            PrintWriter w = new PrintWriter(file, "UTF-8");
+            AtomicFileWriter afw = new AtomicFileWriter(file);
             try {
+                PrintWriter w = new PrintWriter(afw);
                 w.println("<?xml version='1.0' encoding='UTF-8'?>");
                 w.println("<fingerprint>");
                 w.print("  <timestamp>");
@@ -1139,8 +1146,9 @@ void save(File file) throws IOException {
                 w.println("  <facets/>");
                 w.print("</fingerprint>");
                 w.flush();
+                afw.commit();
             } finally {
-                w.close();
+                afw.abort();
             }
         } else {
             // Slower fallback that can persist facets.
@@ -1230,7 +1238,7 @@ private static File getFingerprintFile(byte[] md5sum) {
                 file.delete();
                 return null;
             }
-            String parseError = messageOfXmlPullParserException(e);
+            String parseError = messageOfParseException(e);
             if (parseError != null) {
                 logger.log(Level.WARNING, "Malformed XML in {0}: {1}", new Object[] {configFile, parseError});
                 file.delete();
@@ -1240,13 +1248,13 @@ private static File getFingerprintFile(byte[] md5sum) {
             throw e;
         }
     }
-    private static String messageOfXmlPullParserException(Throwable t) {
-        if (t instanceof XmlPullParserException) {
+    private static String messageOfParseException(Throwable t) {
+        if (t instanceof XmlPullParserException || t instanceof EOFException) {
             return t.getMessage();
         }
         Throwable t2 = t.getCause();
         if (t2 != null) {
-            return messageOfXmlPullParserException(t2);
+            return messageOfParseException(t2);
         } else {
             return null;
         }

diff --git a/core/src/main/java/hudson/model/FingerprintCleanupThread.java b/core/src/main/java/hudson/model/FingerprintCleanupThread.java
@@ -101,11 +101,13 @@ private boolean check(File fingerprintFile) {
         try {
             Fingerprint fp = Fingerprint.load(fingerprintFile);
             if (fp == null || !fp.isAlive()) {
+                logger.fine("deleting obsolete " + fingerprintFile);
                 fingerprintFile.delete();
                 return true;
             } else {
                 // get the fingerprint in the official map so have the changes visible to Jenkins
                 // otherwise the mutation made in FingerprintMap can override our trimming.
+                logger.finer("possibly trimming " + fingerprintFile);
                 fp = Jenkins.getInstance()._getFingerprint(fp.getHashString());
                 return fp.trim();
             }