From b782518caedc7b20e3d568f78a5a1d2b0aa17424 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 8 Jun 2018 09:42:13 +0200 Subject: [PATCH 1/3] Refactor FileRepository.detectIndexChange() Change-Id: I50c751e2e90f685dc27043c569da2eb210d4611b Signed-off-by: Matthias Sohn --- .../jgit/internal/storage/file/FileRepository.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java index 13ef94b89..d4056871b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java @@ -557,15 +557,15 @@ public class FileRepository extends Repository { try { if (snapshot == null) { snapshot = FileSnapshot.save(indexFile); - } else if (snapshot.isModified(indexFile)) { - snapshotLock.unlock(); - notifyIndexChanged(false); + return; } - } finally { - if (snapshotLock.isHeldByCurrentThread()) { - snapshotLock.unlock(); + if (!snapshot.isModified(indexFile)) { + return; } + } finally { + snapshotLock.unlock(); } + notifyIndexChanged(false); } /** {@inheritDoc} */ From 1cb8c5d7fe2d88c127bafcff3800b91e5ab5eda4 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 8 Jun 2018 09:50:39 +0200 Subject: [PATCH 2/3] Simplify locking of FileRepository's index snapshot synchronize on simple Object monitor instead of using ReentrantLock Change-Id: I897020ab35786336b51b0fef76ea6071aff8aefa Signed-off-by: Matthias Sohn --- .../jgit/internal/storage/file/FileRepository.java | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java index d4056871b..d02888a87 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java @@ -56,7 +56,6 @@ import java.util.HashSet; import java.util.Locale; import java.util.Objects; import java.util.Set; -import java.util.concurrent.locks.ReentrantLock; import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.api.errors.JGitInternalException; @@ -125,7 +124,7 @@ public class FileRepository extends Repository { private final RefDatabase refs; private final ObjectDirectory objectDatabase; - private final ReentrantLock snapshotLock = new ReentrantLock(); + private final Object snapshotLock = new Object(); // protected by snapshotLock private FileSnapshot snapshot; @@ -553,8 +552,7 @@ public class FileRepository extends Repository { } File indexFile = getIndexFile(); - snapshotLock.lock(); - try { + synchronized (snapshotLock) { if (snapshot == null) { snapshot = FileSnapshot.save(indexFile); return; @@ -562,8 +560,6 @@ public class FileRepository extends Repository { if (!snapshot.isModified(indexFile)) { return; } - } finally { - snapshotLock.unlock(); } notifyIndexChanged(false); } @@ -571,11 +567,8 @@ public class FileRepository extends Repository { /** {@inheritDoc} */ @Override public void notifyIndexChanged(boolean internal) { - snapshotLock.lock(); - try { + synchronized (snapshotLock) { snapshot = FileSnapshot.save(getIndexFile()); - } finally { - snapshotLock.unlock(); } fireEvent(new IndexChangedEvent(internal)); } From 5f27032fb85694a093f827581216d4ffb99db68b Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 8 Jun 2018 17:22:54 +0200 Subject: [PATCH 3/3] Use constant for ".lock" Change-Id: Id65dc94c970ffd3ca3d3d4a5d57123c95d29e8af Signed-off-by: Matthias Sohn --- .../jgit/internal/storage/file/RefUpdateTest.java | 11 ++++++----- .../eclipse/jgit/internal/storage/file/LockFile.java | 8 +++++--- .../jgit/internal/storage/file/ReflogWriter.java | 3 ++- .../src/org/eclipse/jgit/lib/Constants.java | 7 +++++++ .../src/org/eclipse/jgit/lib/Repository.java | 8 ++++++-- .../src/org/eclipse/jgit/transport/TransportSftp.java | 4 +++- 6 files changed, 29 insertions(+), 12 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java index 757ebb298..b2fae316c 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java @@ -45,8 +45,9 @@ package org.eclipse.jgit.internal.storage.file; -import static org.eclipse.jgit.lib.Constants.CHARSET; import static org.eclipse.jgit.junit.Assert.assertEquals; +import static org.eclipse.jgit.lib.Constants.CHARSET; +import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -819,11 +820,11 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase { // Check that the involved refs are the same despite the failure assertExists(false, toName); if (!toLock.equals(toName)) - assertExists(false, toName + ".lock"); - assertExists(true, toLock + ".lock"); + assertExists(false, toName + LOCK_SUFFIX); + assertExists(true, toLock + LOCK_SUFFIX); if (!toLock.equals(fromName)) - assertExists(false, "logs/" + fromName + ".lock"); - assertExists(false, "logs/" + toName + ".lock"); + assertExists(false, "logs/" + fromName + LOCK_SUFFIX); + assertExists(false, "logs/" + toName + LOCK_SUFFIX); assertEquals(oldHeadId, db.resolve(Constants.HEAD)); assertEquals(oldfromId, db.resolve(fromName)); assertNull(db.resolve(toName)); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java index a11e4ffdf..da9a050bc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java @@ -44,6 +44,8 @@ package org.eclipse.jgit.internal.storage.file; +import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX; + import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -74,7 +76,6 @@ import org.eclipse.jgit.util.FileUtils; * name. */ public class LockFile { - static final String SUFFIX = ".lock"; //$NON-NLS-1$ /** * Unlock the given file. @@ -106,14 +107,15 @@ public class LockFile { * @return lock file */ static File getLockFile(File file) { - return new File(file.getParentFile(), file.getName() + SUFFIX); + return new File(file.getParentFile(), + file.getName() + LOCK_SUFFIX); } /** Filter to skip over active lock files when listing a directory. */ static final FilenameFilter FILTER = new FilenameFilter() { @Override public boolean accept(File dir, String name) { - return !name.endsWith(SUFFIX); + return !name.endsWith(LOCK_SUFFIX); } }; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java index b366eafe2..495c75236 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java @@ -46,6 +46,7 @@ package org.eclipse.jgit.internal.storage.file; import static org.eclipse.jgit.lib.Constants.HEAD; +import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX; import static org.eclipse.jgit.lib.Constants.R_HEADS; import static org.eclipse.jgit.lib.Constants.R_NOTES; import static org.eclipse.jgit.lib.Constants.R_REFS; @@ -84,7 +85,7 @@ public class ReflogWriter { * @return the name of the ref's lock ref. */ public static String refLockFor(String name) { - return name + LockFile.SUFFIX; + return name + LOCK_SUFFIX; } private final RefDirectory refdb; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java index a119480ea..ecebd5408 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java @@ -691,6 +691,13 @@ public final class Constants { public static final ObjectId EMPTY_BLOB_ID = ObjectId .fromString("e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"); + /** + * Suffix of lock file name + * + * @since 5.0 + */ + public static final String LOCK_SUFFIX = ".lock"; //$NON-NLS-1$ + private Constants() { // Hide the default constructor } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index fa283d012..29cc19c43 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -48,6 +48,8 @@ package org.eclipse.jgit.lib; +import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX; + import java.io.BufferedOutputStream; import java.io.File; import java.io.FileNotFoundException; @@ -1366,10 +1368,12 @@ public abstract class Repository implements AutoCloseable { */ public static boolean isValidRefName(String refName) { final int len = refName.length(); - if (len == 0) + if (len == 0) { return false; - if (refName.endsWith(".lock")) //$NON-NLS-1$ + } + if (refName.endsWith(LOCK_SUFFIX)) { return false; + } // Refs may be stored as loose files so invalid paths // on the local system must also be invalid refs. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportSftp.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportSftp.java index d8f3a12bf..f129ba34d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportSftp.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportSftp.java @@ -43,6 +43,8 @@ package org.eclipse.jgit.transport; +import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX; + import java.io.BufferedReader; import java.io.FileNotFoundException; import java.io.IOException; @@ -344,7 +346,7 @@ public class TransportSftp extends SshTransport implements WalkTransport { @Override void writeFile(String path, byte[] data) throws IOException { - final String lock = path + ".lock"; //$NON-NLS-1$ + final String lock = path + LOCK_SUFFIX; try { super.writeFile(lock, data); try {