From fef782128d35777cc8e57dd13744e54689305089 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Wed, 6 Mar 2019 00:31:45 +0000 Subject: [PATCH] Do not reuse packfiles when changed on filesystem The pack reload mechanism from the filesystem works only by name and does not check the actual last modified date of the packfile. This lead to concurrency issues where multiple threads were loading and removing from each other list of packfiles when one of those was failing the checksum. Rely on FileSnapshot rather than directly checking lastModified timestamp so that more checks can be performed. Bug: 544199 Change-Id: I173328f29d9914007fd5eae3b4c07296ab292390 Signed-off-by: Luca Milanesio --- .../jgit/internal/storage/file/ObjectDirectory.java | 4 ++-- .../jgit/internal/storage/file/PackFile.java | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java index 6489415eb..44ad99bb9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java @@ -820,13 +820,13 @@ private PackList scanPacksImpl(final PackList old) { } final String packName = base + PACK.getExtension(); + final File packFile = new File(packDirectory, packName); final PackFile oldPack = forReuse.remove(packName); - if (oldPack != null) { + if (oldPack != null && oldPack.getFileSnapshot().isModified(packFile)) { list.add(oldPack); continue; } - final File packFile = new File(packDirectory, packName); list.add(new PackFile(packFile, extensions)); foundNew = true; } 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 b5889f2dd..9a1d12f40 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 @@ -129,6 +129,8 @@ public int compare(final PackFile a, final PackFile b) { int packLastModified; + private FileSnapshot fileSnapshot; + private volatile boolean invalid; private boolean invalidBitmap; @@ -163,6 +165,7 @@ public int compare(final PackFile a, final PackFile b) { public PackFile(final File packFile, int extensions) { this.packFile = packFile; this.packLastModified = (int) (packFile.lastModified() >> 10); + this.fileSnapshot = FileSnapshot.save(packFile); this.extensions = extensions; // Multiply by 31 here so we can more directly combine with another @@ -335,6 +338,16 @@ ObjectId findObjectForOffset(final long offset) throws IOException { return getReverseIdx().findObject(offset); } + /** + * Return the @{@link FileSnapshot} associated to the underlying packfile + * that has been used when the object was created. + * + * @return the packfile @{@link FileSnapshot} that the object is loaded from. + */ + FileSnapshot getFileSnapshot() { + return fileSnapshot; + } + private final byte[] decompress(final long position, final int sz, final WindowCursor curs) throws IOException, DataFormatException { byte[] dstbuf;