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 <luca.milanesio@gmail.com>
This commit is contained in:
parent
d4078dccda
commit
fef782128d
|
@ -820,13 +820,13 @@ private PackList scanPacksImpl(final PackList old) {
|
||||||
}
|
}
|
||||||
|
|
||||||
final String packName = base + PACK.getExtension();
|
final String packName = base + PACK.getExtension();
|
||||||
|
final File packFile = new File(packDirectory, packName);
|
||||||
final PackFile oldPack = forReuse.remove(packName);
|
final PackFile oldPack = forReuse.remove(packName);
|
||||||
if (oldPack != null) {
|
if (oldPack != null && oldPack.getFileSnapshot().isModified(packFile)) {
|
||||||
list.add(oldPack);
|
list.add(oldPack);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
final File packFile = new File(packDirectory, packName);
|
|
||||||
list.add(new PackFile(packFile, extensions));
|
list.add(new PackFile(packFile, extensions));
|
||||||
foundNew = true;
|
foundNew = true;
|
||||||
}
|
}
|
||||||
|
|
|
@ -129,6 +129,8 @@ public int compare(final PackFile a, final PackFile b) {
|
||||||
|
|
||||||
int packLastModified;
|
int packLastModified;
|
||||||
|
|
||||||
|
private FileSnapshot fileSnapshot;
|
||||||
|
|
||||||
private volatile boolean invalid;
|
private volatile boolean invalid;
|
||||||
|
|
||||||
private boolean invalidBitmap;
|
private boolean invalidBitmap;
|
||||||
|
@ -163,6 +165,7 @@ public int compare(final PackFile a, final PackFile b) {
|
||||||
public PackFile(final File packFile, int extensions) {
|
public PackFile(final File packFile, int extensions) {
|
||||||
this.packFile = packFile;
|
this.packFile = packFile;
|
||||||
this.packLastModified = (int) (packFile.lastModified() >> 10);
|
this.packLastModified = (int) (packFile.lastModified() >> 10);
|
||||||
|
this.fileSnapshot = FileSnapshot.save(packFile);
|
||||||
this.extensions = extensions;
|
this.extensions = extensions;
|
||||||
|
|
||||||
// Multiply by 31 here so we can more directly combine with another
|
// 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 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,
|
private final byte[] decompress(final long position, final int sz,
|
||||||
final WindowCursor curs) throws IOException, DataFormatException {
|
final WindowCursor curs) throws IOException, DataFormatException {
|
||||||
byte[] dstbuf;
|
byte[] dstbuf;
|
||||||
|
|
Loading…
Reference in New Issue