From: "Shawn O. Pearce" <spearce@spearce.org>
To: Robin Rosenberg <robin.rosenberg@dewire.com>
Cc: git@vger.kernel.org
Subject: [JGIT PATCH 1/2] Ensure that a PackFile instance stays invalid when overwritten
Date: Sat, 2 May 2009 13:30:29 -0700 [thread overview]
Message-ID: <1241296230-19342-1-git-send-email-spearce@spearce.org> (raw)
If a PackFile gets overwritten after we read its pack-*.idx we
throw a PackMismatchException and then close the PackFile, which
discards the index. At this point the PackFile instance is dead
and must be discarded by the GC, as it is used as part of the key
in the WindowCache and the UnpackedObjectCache.
There is however a subtle race condition here. If the PackFile is
opened again after the PackMismatchException is thrown we load the
new index file into memory, possibly seeing the footer in the index
match the footer in the pack, and believing that the file is valid.
This can mean that stale windows that haven't yet expired out of the
WindowCache suddenly become valid again, even though they contain
data from the old version of the pack.
By caching the pack signature the very first time we open it we
can more reliably detect this overwrite race condition and keep
the PackFile instance invalid.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/lib/PackFile.java | 18 ++++++++++++++++--
1 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
index b107dfe..85690a7 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
@@ -89,6 +89,8 @@ public int compare(final PackFile a, final PackFile b) {
private volatile boolean invalid;
+ private byte[] packChecksum;
+
private PackIndex loadedIdx;
private PackReverseIndex reverseIdx;
@@ -115,8 +117,18 @@ public PackFile(final File idxFile, final File packFile) {
private synchronized PackIndex idx() throws IOException {
if (loadedIdx == null) {
+ if (invalid)
+ throw new PackMismatchException("Pack checksum mismatch");
+
try {
- loadedIdx = PackIndex.open(idxFile);
+ final PackIndex idx = PackIndex.open(idxFile);
+
+ if (packChecksum == null)
+ packChecksum = idx.packChecksum;
+ else if (!Arrays.equals(packChecksum, idx.packChecksum))
+ throw new PackMismatchException("Pack checksum mismatch");
+
+ loadedIdx = idx;
} catch (IOException e) {
invalid = true;
throw e;
@@ -339,6 +351,8 @@ synchronized boolean endWindowCache() {
private void doOpen() throws IOException {
try {
+ if (invalid)
+ throw new PackMismatchException("Pack checksum mismatch");
fd = new RandomAccessFile(packFile, "r");
length = fd.length();
onOpenPack();
@@ -423,7 +437,7 @@ private void onOpenPack() throws IOException {
+ ": " + getPackFile());
NB.readFully(fd.getChannel(), length - 20, buf, 0, 20);
- if (!Arrays.equals(buf, idx.packChecksum))
+ if (!Arrays.equals(buf, packChecksum))
throw new PackMismatchException("Pack checksum mismatch:"
+ " pack " + ObjectId.fromRaw(buf).name()
+ " index " + ObjectId.fromRaw(idx.packChecksum).name()
--
1.6.3.rc3.212.g8c698
next reply other threads:[~2009-05-02 20:30 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-02 20:30 Shawn O. Pearce [this message]
2009-05-02 20:30 ` [JGIT PATCH 2/2] Correct Javadoc comment for TransportLocal about forking Shawn O. Pearce
2009-05-03 7:52 ` Robin Rosenberg
2009-05-04 13:57 ` Shawn O. Pearce
2009-05-03 8:05 ` [JGIT PATCH 1/2] Ensure that a PackFile instance stays invalid when overwritten Robin Rosenberg
2009-05-04 19:30 ` [JGIT PATCH v2 " Shawn O. Pearce
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1241296230-19342-1-git-send-email-spearce@spearce.org \
--to=spearce@spearce.org \
--cc=git@vger.kernel.org \
--cc=robin.rosenberg@dewire.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).