Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jgit-dev] jgit and huge files

On 20.03.2012 15:07, Shawn Pearce wrote:
> On Tue, Mar 20, 2012 at 04:19, Marc Strapetz <marc.strapetz@xxxxxxxxxxx> wrote:
>> On 19.03.2012 21:24, Shawn Pearce wrote:
>>> On Mon, Mar 19, 2012 at 12:38, Marc Strapetz <marc.strapetz@xxxxxxxxxxx> wrote:
>>>> Here is the stack trace (we have some debug code in PackFile which is
>>>> definitely not related, hence I transformed only the relevant line
>>>> number). Should "int sz" be a "long sz" instead?
>>
>>> java.lang.NegativeArraySizeException
>>>        at org.eclipse.jgit.storage.file.PackFile.decompress(PackFile.java:296)
>>>        at org.eclipse.jgit.storage.file.PackFile.load(PackFile.java:697)
> 
> Ick. Line 697 is the whole (non-delta) code path. We shouldn't have
> had 2.2G less than the streamFileThreshold:
> 
> 					if (sz < curs.getStreamFileThreshold())
> 						data = decompress(pos + p, (int) sz, curs);
> 
> sz here is a long and came from a loop higher up.
> curs.getStreamFileThreshold() is an int and its maximum value should
> be ~2047m, or 2^31-1. I expected the curs.getStreamFileTreshold() to
> be upcast to a long, and for both longs to be positive when this
> conditional is tested. If this is true then it should be safe to
> truncate sz down from long to int because it is smaller an int's
> maximum positive value.
> 
> Unfortunately something isn't holding true here. If you can add debug
> code to examine "sz" and curs.getStreamFileTreshold()" at this line it
> would be interesting to see what they actually are before the compare
> starts. This would go very badly if sz was already negative for
> example. A negative sz value indicates the pack data may be corrupt
> here for this object because JGit wasn't able to decode the sz from
> the packed object header in the loop above on line 685.

We got it now: attached you will find both of my modifications to
PackFile.java. The more recent commit shows at which places I have added
debug logging. Tail of the output is:

PackFile.load
curs = org.eclipse.jgit.storage.file.WindowCursor@10849b63
pos = 2409357151
c = 6
typeCode = 2
sz = 109
shift = 11
p = 2
PackFile.load
curs = org.eclipse.jgit.storage.file.WindowCursor@10849b63
pos = 2409357265
c = 2
typeCode = 2
sz = 32
shift = 11
p = 2
PackFile.load
curs = org.eclipse.jgit.storage.file.WindowCursor@10849b63
pos = 2409357306
c = 4
typeCode = 2
sz = 73
shift = 11
p = 2
PackFile.load
curs = org.eclipse.jgit.storage.file.WindowCursor@10849b63
pos = 2418223649
c = 8
typeCode = 2
sz = 136
shift = 11
p = 2
PackFile.load
curs = org.eclipse.jgit.storage.file.WindowCursor@6c9122c5
pos = 2409357151
c = 6
typeCode = 2
sz = 109
shift = 11
p = 2
PackFile.load
curs = org.eclipse.jgit.storage.file.WindowCursor@6c9122c5
pos = 2409357265
c = 2
typeCode = 2
sz = 32
shift = 11
p = 2
PackFile.load
curs = org.eclipse.jgit.storage.file.WindowCursor@6c9122c5
pos = 2409357306
c = 4
typeCode = 2
sz = 73
shift = 11
p = 2
PackFile.load
curs = org.eclipse.jgit.storage.file.WindowCursor@6c9122c5
pos = 2418223649
c = 8
typeCode = 2
sz = 136
shift = 11
p = 2
PackFile.load
curs = org.eclipse.jgit.storage.file.WindowCursor@2c81eb32
pos = 2418223760
c = 68
typeCode = 3
sz = -2007016313
shift = 32
p = 5
java.lang.RuntimeException:
position=2418223765;sz=-2007016313;curs.getStreamFileThreshold()=10485760
     at org.eclipse.jgit.storage.file.PackFile.decompress(PackFile.java:323)
     at org.eclipse.jgit.storage.file.PackFile.load(PackFile.java:729)
     at org.eclipse.jgit.storage.file.PackFile.get(PackFile.java:237)

-Marc




commit 3d04d0d58762f3e65813b2e44f3753123648146b
Author: Marc Strapetz <marc.strapetz@xxxxxxxxxxx>
Date:   Tue Mar 20 17:31:45 2012 +0100

    PackFile: debug code added.

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/PackFile.java
index 472f536..4432d8c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/PackFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/PackFile.java
@@ -125,6 +125,7 @@ public int compare(final PackFile a, final PackFile b) {
 
 	private static Map<PackFile, Exception> openPackToStackTrace = new HashMap<PackFile, Exception>();
 	private static boolean DEBUG_OPEN_PACKS = Boolean.getBoolean("jgit.debug-open-packs");
+	private static boolean TRACE_BUG = Boolean.getBoolean("jgit.tracePackFileBug");
 
 	public static void outputOpenPacks() {
 		for (PackFile pack : openPackToStackTrace.keySet()) {
@@ -318,6 +319,8 @@ ObjectId findObjectForOffset(final long offset) throws IOException {
 			// maximum heap size is only 256 MB. Even if the JRE has
 			// 200 MB free, it cannot allocate a 640 MB byte array.
 			return null;
+		} catch (NegativeArraySizeException nase) {
+			throw new RuntimeException("position=" + position + ";sz=" + sz + ";curs.getStreamFileThreshold()=" + curs.getStreamFileThreshold());
 		}
 
 		if (curs.inflate(this, position, dstbuf, 0) != sz)
@@ -683,6 +686,12 @@ private void onOpenPack() throws IOException {
 
 	ObjectLoader load(final WindowCursor curs, long pos)
 			throws IOException {
+		if (TRACE_BUG) {
+			System.out.println("PackFile.load");
+			System.out.println("curs = " + curs);
+			System.out.println("pos = " + pos);
+		}
+
 		try {
 			final byte[] ib = curs.tempId;
 			Delta delta = null;
@@ -703,6 +712,14 @@ ObjectLoader load(final WindowCursor curs, long pos)
 					shift += 7;
 				}
 
+				if (TRACE_BUG) {
+					System.out.println("c = " + c);
+					System.out.println("typeCode = " + typeCode);
+					System.out.println("sz = " + sz);
+					System.out.println("shift = " + shift);
+					System.out.println("p = " + p);
+				}
+
 				switch (typeCode) {
 				case Constants.OBJ_COMMIT:
 				case Constants.OBJ_TREE:

commit f524bfaf21a5d72087fe400413dad6bab489ef1c
Author: Marc Strapetz <marc.strapetz@xxxxxxxxxxx>
Date:   Thu May 19 17:03:10 2011 +0200

    PackFile: utilitiy code to debug open packs added.

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/PackFile.java
index 80a0966..472f536 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/PackFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/PackFile.java
@@ -45,18 +45,11 @@
 
 package org.eclipse.jgit.storage.file;
 
-import java.io.EOFException;
-import java.io.File;
-import java.io.IOException;
-import java.io.RandomAccessFile;
+import java.io.*;
 import java.nio.MappedByteBuffer;
 import java.nio.channels.FileChannel.MapMode;
 import java.text.MessageFormat;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.Iterator;
-import java.util.Set;
+import java.util.*;
 import java.util.zip.CRC32;
 import java.util.zip.DataFormatException;
 import java.util.zip.Inflater;
@@ -130,6 +123,16 @@ public int compare(final PackFile a, final PackFile b) {
 	 */
 	private volatile LongList corruptObjects;
 
+	private static Map<PackFile, Exception> openPackToStackTrace = new HashMap<PackFile, Exception>();
+	private static boolean DEBUG_OPEN_PACKS = Boolean.getBoolean("jgit.debug-open-packs");
+
+	public static void outputOpenPacks() {
+		for (PackFile pack : openPackToStackTrace.keySet()) {
+			System.err.println(pack + " " + pack.idxFile);
+			openPackToStackTrace.get(pack).printStackTrace();
+		}
+	}
+
 	/**
 	 * Construct a reader for an existing, pre-indexed packfile.
 	 *
@@ -148,6 +151,12 @@ public PackFile(final File idxFile, final File packFile) {
 		//
 		hash = System.identityHashCode(this) * 31;
 		length = Long.MAX_VALUE;
+
+		if (DEBUG_OPEN_PACKS) {
+			synchronized (PackFile.class) {
+				openPackToStackTrace.put(this, new Exception());
+			}
+		}
 	}
 
 	private synchronized PackIndex idx() throws IOException {
@@ -236,6 +245,12 @@ void resolve(Set<ObjectId> matches, AbbreviatedObjectId id, int matchLimit)
 	 * Close the resources utilized by this repository
 	 */
 	public void close() {
+		if (DEBUG_OPEN_PACKS) {
+			synchronized (PackFile.class) {
+				openPackToStackTrace.remove(this);
+			}
+		}
+
 		WindowCache.purge(this);
 		synchronized (this) {
 			loadedIdx = null;


Back to the top