From 391c7a25fa8d247f9f6087f9f19ee330540ad602 Mon Sep 17 00:00:00 2001 From: Michael Keppler Date: Sat, 9 Mar 2019 12:37:10 +0100 Subject: Avoid NPE in ObjectId.isId() That method can easily be invoked with a null argument (e.g. isId(repo.getFullBranch()), therefore it should handle null arguments. Change was suggested in https://git.eclipse.org/r/#/c/137918/, which tried to fix the same in egit only. Bug:544982 Change-Id: I32d1df6e9b2946ab324eda7008721159019316b3 Signed-off-by: Michael Keppler --- org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectId.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'org.eclipse.jgit') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectId.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectId.java index 764f890158..0e96138c00 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectId.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectId.java @@ -49,6 +49,7 @@ import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.Serializable; +import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.errors.InvalidObjectIdException; import org.eclipse.jgit.util.NB; import org.eclipse.jgit.util.RawParseUtils; @@ -86,7 +87,10 @@ public class ObjectId extends AnyObjectId implements Serializable { * the string to test. * @return true if the string can converted into an ObjectId. */ - public static final boolean isId(String id) { + public static final boolean isId(@Nullable String id) { + if (id == null) { + return false; + } if (id.length() != Constants.OBJECT_ID_STRING_LENGTH) return false; try { -- cgit v1.2.3 From 19c43806890a5b7556d4d12b87f7ba2bb878cbf8 Mon Sep 17 00:00:00 2001 From: Juergen Denner Date: Tue, 15 Jan 2019 13:22:54 +0100 Subject: Reduce contention on PackFile.idx() function. In case of concurrent pack file access, threads may wait on the idx() function even for already open files. This happens especially with a slow file system. Performance numbers are listed in the bug report. Bug: 543739 Change-Id: Iff328d347fa65ae07ecce3267d44184161248978 Signed-off-by: Juergen Denner Signed-off-by: Matthias Sohn --- .../jgit/internal/storage/file/PackFile.java | 61 +++++++++++++--------- 1 file changed, 35 insertions(+), 26 deletions(-) (limited to 'org.eclipse.jgit') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java index d28c04fd64..3e5ef21cce 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java @@ -139,7 +139,7 @@ public class PackFile implements Iterable { private byte[] packChecksum; - private PackIndex loadedIdx; + private volatile PackIndex loadedIdx; private PackReverseIndex reverseIdx; @@ -174,35 +174,44 @@ public class PackFile implements Iterable { length = Long.MAX_VALUE; } - private synchronized PackIndex idx() throws IOException { - if (loadedIdx == null) { - if (invalid) - throw new PackInvalidException(packFile); - - try { - final PackIndex idx = PackIndex.open(extFile(INDEX)); - - if (packChecksum == null) { - packChecksum = idx.packChecksum; - } else if (!Arrays.equals(packChecksum, idx.packChecksum)) { - throw new PackMismatchException(MessageFormat.format( - JGitText.get().packChecksumMismatch, - packFile.getPath(), - ObjectId.fromRaw(packChecksum).name(), - ObjectId.fromRaw(idx.packChecksum).name())); + private PackIndex idx() throws IOException { + PackIndex idx = loadedIdx; + if (idx == null) { + synchronized (this) { + idx = loadedIdx; + if (idx == null) { + if (invalid) { + throw new PackInvalidException(packFile); + } + try { + idx = PackIndex.open(extFile(INDEX)); + + if (packChecksum == null) { + packChecksum = idx.packChecksum; + } else if (!Arrays.equals(packChecksum, + idx.packChecksum)) { + throw new PackMismatchException(MessageFormat + .format(JGitText.get().packChecksumMismatch, + packFile.getPath(), + ObjectId.fromRaw(packChecksum) + .name(), + ObjectId.fromRaw(idx.packChecksum) + .name())); + } + loadedIdx = idx; + } catch (InterruptedIOException e) { + // don't invalidate the pack, we are interrupted from + // another thread + throw e; + } catch (IOException e) { + invalid = true; + throw e; + } } - loadedIdx = idx; - } catch (InterruptedIOException e) { - // don't invalidate the pack, we are interrupted from another thread - throw e; - } catch (IOException e) { - invalid = true; - throw e; } } - return loadedIdx; + return idx; } - /** @return the File object which locates this pack on disk. */ public File getPackFile() { return packFile; -- cgit v1.2.3