[
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;