Retry stale file handles on .git/config file

On a local non-NFS filesystem the .git/config file will be orphaned if
it is replaced by a new process while the current process is reading the
old file. The current process successfully continues to read the
orphaned file until it closes the file handle.

Since NFS servers do not keep track of open files, instead of orphaning
the old .git/config file, such a replacement on an NFS filesystem will
instead cause the old file to be garbage collected (deleted).  A stale
file handle exception will be raised on NFS clients if the file is
garbage collected (deleted) on the server while it is being read.  Since
we no longer have access to the old file in these cases, the previous
code would just fail. However, in these cases, reopening the file and
rereading it will succeed (since it will open the new replacement file).
Since retrying the read is a viable strategy to deal with stale file
handles on the .git/config file, implement such a strategy.

Since it is possible that the .git/config file could be replaced again
while rereading it, loop on stale file handle exceptions, up to 5 extra
times, trying to read the .git/config file again, until we either read
the new file, or find that the file no longer exists. The limit of 5 is
arbitrary, and provides a safe upper bounds to prevent infinite loops
consuming resources in a potential unforeseen persistent error
condition.

Change-Id: I6901157b9dfdbd3013360ebe3eb40af147a8c626
Signed-off-by: Nasser Grainawi <nasser@codeaurora.org>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

3 files changed, 56 insertions, 31 deletions

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index b5b04b4743..c949c0aa9b 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -123,6 +123,7 @@ commitMessageNotSpecified=commit message not specified
 commitOnRepoWithoutHEADCurrentlyNotSupported=Commit on repo without HEAD currently not supported
 commitAmendOnInitialNotPossible=Amending is not possible on initial commit.
 compressingObjects=Compressing objects
+configHandleIsStale=config file handle is stale, {0}. retry
 connectionFailed=connection failed
 connectionTimeOut=Connection time out: {0}
 contextMustBeNonNegative=context must be >= 0
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 758f71d27a..d32e873c19 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -182,6 +182,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String commitOnRepoWithoutHEADCurrentlyNotSupported;
 	/***/ public String commitAmendOnInitialNotPossible;
 	/***/ public String compressingObjects;
+	/***/ public String configHandleIsStale;
 	/***/ public String connectionFailed;
 	/***/ public String connectionTimeOut;
 	/***/ public String contextMustBeNonNegative;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java
index 4b4822c763..9a38846442 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java
@@ -65,13 +65,19 @@ import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.util.FS;
+import org.eclipse.jgit.util.FileUtils;
 import org.eclipse.jgit.util.IO;
 import org.eclipse.jgit.util.RawParseUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * The configuration file that is stored in the file of the file system.
  */
 public class FileBasedConfig extends StoredConfig {
+	private final static Logger LOG = LoggerFactory
+			.getLogger(FileBasedConfig.class);
+
 	private final File configFile;
 
 	private boolean utf8Bom;
@@ -135,41 +141,58 @@ public class FileBasedConfig extends StoredConfig {
 	 */
 	@Override
 	public void load() throws IOException, ConfigInvalidException {
-		final FileSnapshot oldSnapshot = snapshot;
-		final FileSnapshot newSnapshot = FileSnapshot.save(getFile());
-		try {
-			final byte[] in = IO.readFully(getFile());
-			final ObjectId newHash = hash(in);
-			if (hash.equals(newHash)) {
-				if (oldSnapshot.equals(newSnapshot))
-					oldSnapshot.setClean(newSnapshot);
-				else
-					snapshot = newSnapshot;
-			} else {
-				final String decoded;
-				if (isUtf8(in)) {
-					decoded = RawParseUtils.decode(RawParseUtils.UTF8_CHARSET,
-							in, 3, in.length);
-					utf8Bom = true;
+		final int maxStaleRetries = 5;
+		int retries = 0;
+		while (true) {
+			final FileSnapshot oldSnapshot = snapshot;
+			final FileSnapshot newSnapshot = FileSnapshot.save(getFile());
+			try {
+				final byte[] in = IO.readFully(getFile());
+				final ObjectId newHash = hash(in);
+				if (hash.equals(newHash)) {
+					if (oldSnapshot.equals(newSnapshot)) {
+						oldSnapshot.setClean(newSnapshot);
+					} else {
+						snapshot = newSnapshot;
+					}
 				} else {
-					decoded = RawParseUtils.decode(in);
+					final String decoded;
+					if (isUtf8(in)) {
+						decoded = RawParseUtils.decode(
+								RawParseUtils.UTF8_CHARSET, in, 3, in.length);
+						utf8Bom = true;
+					} else {
+						decoded = RawParseUtils.decode(in);
+					}
+					fromText(decoded);
+					snapshot = newSnapshot;
+					hash = newHash;
 				}
-				fromText(decoded);
+				return;
+			} catch (FileNotFoundException noFile) {
+				if (configFile.exists()) {
+					throw noFile;
+				}
+				clear();
 				snapshot = newSnapshot;
-				hash = newHash;
-			}
-		} catch (FileNotFoundException noFile) {
-			if (configFile.exists()) {
-				throw noFile;
+				return;
+			} catch (IOException e) {
+				if (FileUtils.isStaleFileHandle(e)
+						&& retries < maxStaleRetries) {
+					if (LOG.isDebugEnabled()) {
+						LOG.debug(MessageFormat.format(
+								JGitText.get().configHandleIsStale,
+								Integer.valueOf(retries)), e);
+					}
+					retries++;
+					continue;
+				}
+				throw new IOException(MessageFormat
+						.format(JGitText.get().cannotReadFile, getFile()), e);
+			} catch (ConfigInvalidException e) {
+				throw new ConfigInvalidException(MessageFormat
+						.format(JGitText.get().cannotReadFile, getFile()), e);
 			}
-			clear();
-			snapshot = newSnapshot;
-		} catch (IOException e) {
-			final IOException e2 = new IOException(MessageFormat.format(JGitText.get().cannotReadFile, getFile()));
-			e2.initCause(e);
-			throw e2;
-		} catch (ConfigInvalidException e) {
-			throw new ConfigInvalidException(MessageFormat.format(JGitText.get().cannotReadFile, getFile()), e);
 		}
 	}