Browse Source

Only increment mod count if packed-refs file changes

Previously if a packed-refs file was racily clean then there
was a 2.5 second window in which each call to getPackedRefs
would increment the mod count causing a RefsChangedEvent to be
fired since the FileSnapshot would report the file as modified.

If a RefsChangedListener called getRef/getRefs from the
onRefsChanged method then a StackOverflowError could occur
since the stack could be exhausted before the 2.5 second
window expired and the packed-refs file would no longer
report being modified.

Now a SHA-1 is computed of the packed-refs file and the
mod count is only incremented when the packed refs are
successfully set and the id of the new packed-refs file
does not match the id of the old packed-refs file.

Change-Id: I8cab6e5929479ed748812b8598c7628370e79697
stable-2.0
Kevin Sawicki 13 years ago
parent
commit
91f5ce3a15
  1. 32
      org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java
  2. 30
      org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java

32
org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java

@ -60,6 +60,8 @@ import static org.junit.Assert.fail;
import java.io.File; import java.io.File;
import java.io.IOException; import java.io.IOException;
import java.util.Map; import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jgit.events.ListenerHandle; import org.eclipse.jgit.events.ListenerHandle;
import org.eclipse.jgit.events.RefsChangedEvent; import org.eclipse.jgit.events.RefsChangedEvent;
@ -1077,6 +1079,36 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
assertSame(master_p2, refdir.peel(master_p2)); assertSame(master_p2, refdir.peel(master_p2));
} }
@Test
public void testRefsChangedStackOverflow() throws Exception {
final FileRepository newRepo = createBareRepository();
final RefDatabase refDb = newRepo.getRefDatabase();
File packedRefs = new File(newRepo.getDirectory(), "packed-refs");
assertTrue(packedRefs.createNewFile());
final AtomicReference<StackOverflowError> error = new AtomicReference<StackOverflowError>();
final AtomicReference<IOException> exception = new AtomicReference<IOException>();
final AtomicInteger changeCount = new AtomicInteger();
newRepo.getListenerList().addRefsChangedListener(
new RefsChangedListener() {
public void onRefsChanged(RefsChangedEvent event) {
try {
refDb.getRefs("ref");
changeCount.incrementAndGet();
} catch (StackOverflowError soe) {
error.set(soe);
} catch (IOException ioe) {
exception.set(ioe);
}
}
});
refDb.getRefs("ref");
refDb.getRefs("ref");
assertNull(error.get());
assertNull(exception.get());
assertEquals(1, changeCount.get());
}
private void writeLooseRef(String name, AnyObjectId id) throws IOException { private void writeLooseRef(String name, AnyObjectId id) throws IOException {
writeLooseRef(name, id.name() + "\n"); writeLooseRef(name, id.name() + "\n");
} }

30
org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java

@ -63,6 +63,8 @@ import java.io.FileInputStream;
import java.io.FileNotFoundException; import java.io.FileNotFoundException;
import java.io.IOException; import java.io.IOException;
import java.io.InputStreamReader; import java.io.InputStreamReader;
import java.security.DigestInputStream;
import java.security.MessageDigest;
import java.text.MessageFormat; import java.text.MessageFormat;
import java.util.Arrays; import java.util.Arrays;
import java.util.LinkedList; import java.util.LinkedList;
@ -626,24 +628,27 @@ public class RefDirectory extends RefDatabase {
return curList; return curList;
final PackedRefList newList = readPackedRefs(); final PackedRefList newList = readPackedRefs();
if (packedRefs.compareAndSet(curList, newList)) if (packedRefs.compareAndSet(curList, newList)
&& !curList.id.equals(newList.id))
modCnt.incrementAndGet(); modCnt.incrementAndGet();
return newList; return newList;
} }
private PackedRefList readPackedRefs() private PackedRefList readPackedRefs() throws IOException {
throws IOException {
final FileSnapshot snapshot = FileSnapshot.save(packedRefsFile); final FileSnapshot snapshot = FileSnapshot.save(packedRefsFile);
final BufferedReader br; final BufferedReader br;
final MessageDigest digest = Constants.newMessageDigest();
try { try {
br = new BufferedReader(new InputStreamReader(new FileInputStream( br = new BufferedReader(new InputStreamReader(
packedRefsFile), CHARSET)); new DigestInputStream(new FileInputStream(packedRefsFile),
digest), CHARSET));
} catch (FileNotFoundException noPackedRefs) { } catch (FileNotFoundException noPackedRefs) {
// Ignore it and leave the new list empty. // Ignore it and leave the new list empty.
return PackedRefList.NO_PACKED_REFS; return PackedRefList.NO_PACKED_REFS;
} }
try { try {
return new PackedRefList(parsePackedRefs(br), snapshot); return new PackedRefList(parsePackedRefs(br), snapshot,
ObjectId.fromRaw(digest.digest()));
} finally { } finally {
br.close(); br.close();
} }
@ -724,8 +729,9 @@ public class RefDirectory extends RefDatabase {
if (!lck.commit()) if (!lck.commit())
throw new ObjectWritingException(MessageFormat.format(JGitText.get().unableToWrite, name)); throw new ObjectWritingException(MessageFormat.format(JGitText.get().unableToWrite, name));
packedRefs.compareAndSet(oldPackedList, new PackedRefList( byte[] digest = Constants.newMessageDigest().digest(content);
refs, lck.getCommitSnapshot())); packedRefs.compareAndSet(oldPackedList, new PackedRefList(refs,
lck.getCommitSnapshot(), ObjectId.fromRaw(digest)));
} }
}.writePackedRefs(); }.writePackedRefs();
} }
@ -899,13 +905,17 @@ public class RefDirectory extends RefDatabase {
private static class PackedRefList extends RefList<Ref> { private static class PackedRefList extends RefList<Ref> {
static final PackedRefList NO_PACKED_REFS = new PackedRefList( static final PackedRefList NO_PACKED_REFS = new PackedRefList(
RefList.emptyList(), FileSnapshot.MISSING_FILE); RefList.emptyList(), FileSnapshot.MISSING_FILE,
ObjectId.zeroId());
final FileSnapshot snapshot; final FileSnapshot snapshot;
PackedRefList(RefList<Ref> src, FileSnapshot s) { final ObjectId id;
PackedRefList(RefList<Ref> src, FileSnapshot s, ObjectId i) {
super(src); super(src);
snapshot = s; snapshot = s;
id = i;
} }
} }

Loading…
Cancel
Save