Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNasser Grainawi2018-04-06 20:05:37 +0000
committerMatthias Sohn2018-05-10 09:13:32 +0000
commitd13918310f54cd395dab0d1d0cb83c68b3900cd9 (patch)
tree985e964d0bfa2714bba8e317aa45f0dc3ed564fc
parent7608de5e5db51086c48e90a8d265d12ad965cba8 (diff)
downloadjgit-d13918310f54cd395dab0d1d0cb83c68b3900cd9.tar.gz
jgit-d13918310f54cd395dab0d1d0cb83c68b3900cd9.tar.xz
jgit-d13918310f54cd395dab0d1d0cb83c68b3900cd9.zip
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>
-rw-r--r--org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties1
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java1
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java85
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);
}
}

Back to the top