- Fix several bugs in removal of first or second span

- Fix bugs in flushes
- Add magic numbers for free pages and free list
- More bounds checking
- Lots of checks for double-kill / double-free
- Make some freelist errors non-fatal
- Cleanups, logging, javadocs, test code

core/java/src/net/i2p/client/naming/BlockfileNamingService.java

@@ -450,9 +450,11 @@ public class BlockfileNamingService extends DummyNamingService {
                         nsl.entryAdded(this, hostname, d, options);
                 }
                 return true;
-            } catch (IOException re) {
+            } catch (IOException ioe) {
+                _log.error("DB add error", ioe);
                 return false;
             } catch (RuntimeException re) {
+                _log.error("DB add error", re);
                 return false;
             }
         }
@@ -884,8 +886,8 @@ public class BlockfileNamingService extends DummyNamingService {
     public static void main(String[] args) {
         BlockfileNamingService bns = new BlockfileNamingService(I2PAppContext.getGlobalContext());
         List<String> names = null;
+        Properties props = new Properties();
         try {
-            Properties props = new Properties();
             DataHelper.loadProps(props, new File("hosts.txt"), true);
             names = new ArrayList(props.keySet());
             Collections.shuffle(names);
@@ -911,8 +913,42 @@ public class BlockfileNamingService extends DummyNamingService {
         }
         System.out.println("BFNS took " + DataHelper.formatDuration(System.currentTimeMillis() - start));
         System.out.println("found " + found + " notfound " + notfound);
-        bns.dumpDB();
+
+        System.out.println("Removing all " + names.size() + " hostnames");
+        found = 0;
+        notfound = 0;
+        Collections.shuffle(names);
+        start = System.currentTimeMillis();
+        for (String name : names) {
+             if (bns.remove(name))
+                 found++;
+             else
+                 notfound++;
+        }
+        System.out.println("BFNS took " + DataHelper.formatDuration(System.currentTimeMillis() - start));
+        System.out.println("removed " + found + " not removed " + notfound);
+
+        System.out.println("Adding back " + names.size() + " hostnames");
+        found = 0;
+        notfound = 0;
+        Collections.shuffle(names);
+        start = System.currentTimeMillis();
+        for (String name : names) {
+            try {
+                 if (bns.put(name, new Destination(props.getProperty(name))))
+                     found++;
+                 else
+                     notfound++;
+            } catch (DataFormatException dfe) {}
+        }
+        System.out.println("BFNS took " + DataHelper.formatDuration(System.currentTimeMillis() - start));
+        System.out.println("Added " + found + " not added " + notfound);
+
+
+
+        //bns.dumpDB();
         bns.close();
+        if (true) return;
 
         HostsTxtNamingService htns = new HostsTxtNamingService(I2PAppContext.getGlobalContext());
         found = 0;

core/java/src/net/metanotion/io/block/BlockFile.java

@@ -66,7 +66,7 @@ import net.i2p.util.Log;
  * e.g. the Metaindex skiplist is at offset 1024 bytes
  */
 public class BlockFile {
-	public static final long PAGESIZE = 1024;
+	public static final int PAGESIZE = 1024;
 	public static final long OFFSET_MOUNTED = 20;
 	public static final Log log = I2PAppContext.getGlobalContext().logManager().getLog(BlockFile.class);
 
@@ -79,6 +79,10 @@ public class BlockFile {
 	private static final long MAGIC = MAGIC_BASE | (MAJOR << 8) | MINOR;
 	private long magicBytes = MAGIC;
 	public static final int MAGIC_CONT = 0x434f4e54;   // "CONT"
+	public static final int METAINDEX_PAGE = 2;
+	/** 2**32 pages of 1024 bytes each, more or less */
+	private static final long MAX_LEN = (2l << (32 + 10)) - 1;
+
 	private long fileLen = PAGESIZE * 2;
 	private int freeListStart = 0;
 	private int mounted = 0;
@@ -134,7 +138,7 @@ public class BlockFile {
 		int curPage = page;
 		int dct = 0;
 		while(dct < data.length) {
-			int len = ((int) BlockFile.PAGESIZE) - pageCounter;
+			int len = PAGESIZE - pageCounter;
 			if(len <= 0) {
 				if(curNextPage==0) {
 					curNextPage = this.allocPage();
@@ -150,7 +154,7 @@ public class BlockFile {
 				this.file.skipBytes(4);   // skip magic
 				curNextPage = this.file.readUnsignedInt();
 				pageCounter = 8;
-				len = ((int) BlockFile.PAGESIZE) - pageCounter;
+				len = PAGESIZE - pageCounter;
 			}
 			this.file.write(data, dct, Math.min(len, data.length - dct));
 			pageCounter += Math.min(len, data.length - dct);
@@ -168,7 +172,7 @@ public class BlockFile {
 		int dct = 0;
 		int res;
 		while(dct < arr.length) {
-			int len = ((int) BlockFile.PAGESIZE) - pageCounter;
+			int len = PAGESIZE - pageCounter;
 			if(len <= 0) {
 				BlockFile.pageSeek(this.file, curNextPage);
 				int magic = this.file.readInt();
@@ -177,7 +181,7 @@ public class BlockFile {
 				curPage = curNextPage;
 				curNextPage = this.file.readUnsignedInt();
 				pageCounter = 8;
-				len = ((int) BlockFile.PAGESIZE) - pageCounter;
+				len = PAGESIZE - pageCounter;
 			}
 			res = this.file.read(arr, dct, Math.min(len, arr.length - dct));
 			if(res == -1) { throw new IOException(); }
@@ -221,59 +225,85 @@ public class BlockFile {
 		                              " but actually " + file.length());
 		mount();
 
-		metaIndex = new BSkipList(spanSize, this, 2, new StringBytes(), new IntBytes());
+		metaIndex = new BSkipList(spanSize, this, METAINDEX_PAGE, new StringBytes(), new IntBytes());
 	}
 
-
+	/**
-	public static void pageSeek(RandomAccessInterface file, int page) throws IOException { file.seek((((long)page) - 1L) * BlockFile.PAGESIZE ); }
+	 *  Go to any page but the superblock.
+	 *  Page 1 is the superblock, must use file.seek(0) to get there.
+	 *  @param page >= 2
+	 */
+	public static void pageSeek(RandomAccessInterface file, int page) throws IOException {
+		if (page < METAINDEX_PAGE)
+			throw new IOException("Negative page or superblock access attempt: " + page);
+		file.seek((((long)page) - 1L) * PAGESIZE );
+	}
 
 	public int allocPage() throws IOException {
 		if(freeListStart != 0) {
-			FreeListBlock flb = new FreeListBlock(file, freeListStart);
+			try {
-			if(flb.len > 0) {
+				FreeListBlock flb = new FreeListBlock(file, freeListStart);
-				flb.len = flb.len - 1;
+				if(!flb.isEmpty()) {
-				int page = flb.branches[flb.len];
+					return flb.takePage();
-				flb.writeBlock();
+				} else {
-				return page;
+					freeListStart = flb.getNextPage();
-			} else {
+					writeSuperBlock();
-				freeListStart = flb.nextPage;
+					return flb.page;
-				writeSuperBlock();
+				}
-				return flb.page;
+			} catch (IOException ioe) {
+				log.error("Discarding corrupt free list block page " + freeListStart, ioe);
+				freeListStart = 0;
 			}
 		}
 		long offset = file.length();
-		fileLen = offset + BlockFile.PAGESIZE;
+		fileLen = offset + PAGESIZE;
 		file.setLength(fileLen);
 		writeSuperBlock();
-		return ((int) ((long) (offset / BlockFile.PAGESIZE))) + 1;
+		return (int) ((offset / PAGESIZE) + 1);
 	}
 
-	public void freePage(int page) throws IOException {
+	/**
-		System.out.println("Free Page " + page);
+	 *  Add the page to the free list. The file is never shrunk.
-		if(freeListStart == 0) {
+	 *  TODO: Reclaim free pages at end of file, or even do a full compaction.
-			freeListStart = page;
+	 *  Does not throw exceptions; logs on failure.
-			FreeListBlock.initPage(file, page);
+	 */
-			writeSuperBlock();
+	public void freePage(int page) {
+		if (page < METAINDEX_PAGE) {
+			log.error("Negative page or superblock free attempt: " + page);
 			return;
 		}
-		FreeListBlock flb = new FreeListBlock(file, freeListStart);
+		try {
-		if(flb.isFull()) {
+			if(freeListStart == 0) {
-			FreeListBlock.initPage(file, page);
-			if(flb.nextPage == 0) {
-				flb.nextPage = page;
-				flb.writeBlock();
-				return;
-			} else {
-				flb = new FreeListBlock(file, page);
-				flb.nextPage = freeListStart;
-				flb.writeBlock();
 				freeListStart = page;
+				FreeListBlock.initPage(file, page);
 				writeSuperBlock();
 				return;
 			}
+			try {
+				FreeListBlock flb = new FreeListBlock(file, freeListStart);
+				if(flb.isFull()) {
+					FreeListBlock.initPage(file, page);
+					if(flb.getNextPage() == 0) {
+						flb.setNextPage(page);
+						return;
+					} else {
+						flb = new FreeListBlock(file, page);
+						flb.setNextPage(freeListStart);
+						freeListStart = page;
+						writeSuperBlock();
+						return;
+					}
+				}
+				flb.addPage(page);
+			} catch (IOException ioe) {
+				log.error("Discarding corrupt free list block page " + freeListStart, ioe);
+				freeListStart = page;
+				FreeListBlock.initPage(file, page);
+				writeSuperBlock();
+			}
+		} catch (IOException ioe) {
+			log.error("Error freeing page: " + page, ioe);
 		}
-		flb.addPage(page);
-		flb.writeBlock();
 	}
 
 	public BSkipList getIndex(String name, Serializer key, Serializer val) throws IOException {

core/java/src/net/metanotion/io/block/FreeListBlock.java

@@ -32,21 +32,43 @@ import java.io.IOException;
 
 import net.metanotion.io.RandomAccessInterface;
 
-public class FreeListBlock {
+/**
-	public int page;
+ * On-disk format:
-	public int nextPage;
+ *    Magic number (long)
-	public int len;
+ *    next freelist block page (unsigned int)
-	public int[] branches = null;
+ *    size (unsigned int)
-	public RandomAccessInterface file;
+ *    that many free pages (unsigned ints)
+ *
+ * Always fits on one page.
+ *
+ * Free page format:
+ *    Magic number (long)
+ */
+class FreeListBlock {
+	private static final long MAGIC = 0x2366724c69737423l;  // "#frList#"
+	private static final long MAGIC_FREE = 0x7e2146524545217el;  // "~!FREE!~"
+	private static final int HEADER_LEN = 16;
+	private static final int MAX_SIZE = (BlockFile.PAGESIZE - HEADER_LEN) / 4;
+
+	public final int page;
+	private int nextPage;
+	private int len;
+	private final int[] branches;
+	private final RandomAccessInterface file;
 
 	public FreeListBlock(RandomAccessInterface file, int startPage) throws IOException {
 		this.file = file;
 		this.page = startPage;
 		BlockFile.pageSeek(file, startPage);
+		long magic = file.readLong();
+		if (magic != MAGIC)
+			throw new IOException("Bad freelist magic number 0x" + Long.toHexString(magic) + " on page " + startPage);
 		nextPage = file.readUnsignedInt();
 		len = file.readUnsignedInt();
+		if (len > MAX_SIZE)
+			throw new IOException("Bad freelist size " + len);
+		branches = new int[MAX_SIZE];
 		if(len > 0) {
-			branches = new int[len];
 			for(int i=0;i<len;i++) {
 				branches[i] = file.readUnsignedInt();
 			}
@@ -55,33 +77,102 @@ public class FreeListBlock {
 
 	public void writeBlock() throws IOException {
 		BlockFile.pageSeek(file, page);
+		file.writeLong(MAGIC);
 		file.writeInt(nextPage);
-		if(len > 0) {
+		file.writeInt(len);
-			file.writeInt(len);
+		for(int i=0;i<len;i++) { file.writeInt(branches[i]); }
-			for(int i=0;i<len;i++) { file.writeInt(branches[i]); }
+	}
-		} else {
+
-			file.writeInt(0);
+	/**
-		}
+	 *  Write the length only
+	 */
+	private void writeLen() throws IOException {
+		BlockFile.pageSeek(file, page);
+		file.skipBytes(12);
+		file.writeInt(len);
+	}
+
+	public int getNextPage() {
+		return nextPage;
+	}
+
+	/**
+	 *  Set and write the next page only
+	 */
+	public void setNextPage(int nxt) throws IOException {
+		nextPage = nxt;
+		BlockFile.pageSeek(file, page);
+		file.skipBytes(8);
+		file.writeInt(nxt);
+	}
+
+	/**
+	 *  Write the length and new page only
+	 */
+	private void writeFreePage() throws IOException {
+		BlockFile.pageSeek(file, page);
+		file.skipBytes(12);
+		file.writeInt(len);
+		if (len > 1)
+			file.skipBytes((len - 1) * 4);
+		file.writeInt(branches[len - 1]);
+	}
+
+	public boolean isEmpty() {
+		return len <= 0;
 	}
 
 	public boolean isFull() {
-		int cells = (int) ((BlockFile.PAGESIZE - 8) / 4);
+		return len < MAX_SIZE;
-		if(cells - len > 0) { return false; }
-		return true;
 	}
 
-	public void addPage(int page) {
+	/**
-		int[] t = new int[len + 1];
+	 *  Adds free page and writes new len to disk
-		if(len > 0) {
+	 *  @throws IllegalStateException if full
-			for(int i=0;i<len;i++) { t[i] = branches[i]; }
+	 */
+	public void addPage(int freePage) throws IOException {
+		if (len >= MAX_SIZE)
+			throw new IllegalStateException("full");
+		if (getMagic(freePage) == MAGIC_FREE) {
+			BlockFile.log.error("Double free page " + freePage, new Exception());
+			return;
 		}
-		t[len] = page;
+		branches[len++] = freePage;
-		len++;
+		markFree(freePage);
-		branches = t;
+		writeFreePage();
+	}
+
+	/**
+	 *  Takes next page and writes new len to disk
+	 *  @throws IllegalStateException if empty
+	 */
+	public int takePage() throws IOException {
+		if (len <= 0)
+			throw new IllegalStateException("empty");
+		len--;
+		writeLen();
+		int rv = branches[len];
+		long magic = getMagic(rv);
+		if (magic != MAGIC_FREE)
+			// TODO keep trying until empty
+			throw new IOException("Bad free page magic number 0x" + Long.toHexString(magic) + " on page " + rv);
+		return rv;
+	}
+
+	private void markFree(int freePage) throws IOException {
+		BlockFile.pageSeek(file, freePage);
+		file.writeLong(MAGIC_FREE);
+	}
+
+	private long getMagic(int freePage) throws IOException {
+		BlockFile.pageSeek(file, freePage);
+		long magic = file.readLong();
+		return magic;
 	}
 
 	public static void initPage(RandomAccessInterface file, int page) throws IOException {
 		BlockFile.pageSeek(file, page);
+		file.writeLong(MAGIC);
 		file.writeInt(0);
 		file.writeInt(0);
 	}

core/java/src/net/metanotion/io/block/index/BSkipLevels.java

@@ -36,6 +36,8 @@ import net.metanotion.util.skiplist.SkipList;
 import net.metanotion.util.skiplist.SkipLevels;
 import net.metanotion.util.skiplist.SkipSpan;
 
+import net.i2p.util.Log;
+
 /**
  * On-disk format:
  *    Magic number (long)
@@ -43,12 +45,16 @@ import net.metanotion.util.skiplist.SkipSpan;
  *    non-null height (unsigned short)
  *    span page (unsigned int)
  *    height number of level pages (unsigned ints)
+ *
+ * Always fits on one page.
  */
 public class BSkipLevels extends SkipLevels {
 	private static final long MAGIC = 0x42534c6576656c73l;  // "BSLevels"
+	static final int HEADER_LEN = 16;
 	public final int levelPage;
 	public final int spanPage;
 	public final BlockFile bf;
+	private boolean isKilled;
 
 	public BSkipLevels(BlockFile bf, int levelPage, BSkipList bsl) throws IOException {
 		this.levelPage = levelPage;
@@ -63,11 +69,14 @@ public class BSkipLevels extends SkipLevels {
 
 		int maxLen = bf.file.readUnsignedShort();
 		int nonNull = bf.file.readUnsignedShort();
+		if(maxLen < 1 || maxLen > MAX_SIZE || nonNull > maxLen)
+			throw new IOException("Invalid Level Skip size " + nonNull + " / " + maxLen);
 		spanPage = bf.file.readUnsignedInt();
 		bottom = (BSkipSpan) bsl.spanHash.get(new Integer(spanPage));
 
 		this.levels = new BSkipLevels[maxLen];
-		BlockFile.log.debug("Reading New BSkipLevels with " + nonNull + " valid levels out of " + maxLen + " page " + levelPage);
+		if (BlockFile.log.shouldLog(Log.DEBUG))
+			BlockFile.log.debug("Reading New BSkipLevels with " + nonNull + " / " + maxLen + " valid levels page " + levelPage);
 		// We have to read now because new BSkipLevels() will move the file pointer
 		int[] lps = new int[nonNull];
 		for(int i = 0; i < nonNull; i++) {
@@ -84,7 +93,8 @@ public class BSkipLevels extends SkipLevels {
 				} else {
 				}
 			} else {
-				BlockFile.log.warn("WTF page " + levelPage + " i = " + i + " of " + nonNull + " valid levels out of " + maxLen + " but level page is zero");
+				if (BlockFile.log.shouldLog(Log.WARN))
+					BlockFile.log.warn("WTF " + this + " i = " + i + " of " + nonNull + " / " + maxLen + " valid levels but page is zero");
 				levels[i] = null;
 			}
 		}
@@ -99,6 +109,10 @@ public class BSkipLevels extends SkipLevels {
 	}
 
 	public void flush() {
+		if (isKilled) {
+			BlockFile.log.error("Already killed!! " + this, new Exception());
+			return;
+		}
 		try {
 			BlockFile.pageSeek(bf.file, levelPage);
 			bf.file.writeLong(MAGIC);
@@ -117,9 +131,14 @@ public class BSkipLevels extends SkipLevels {
 	}
 
 	public void killInstance() {
-		try {
+		if (isKilled) {
-			bf.freePage(levelPage);
+			BlockFile.log.error("Already killed!! " + this, new Exception());
-		} catch (IOException ioe) { throw new RuntimeException("Error freeing database page", ioe); }
+			return;
+		}
+		if (BlockFile.log.shouldLog(Log.INFO))
+			BlockFile.log.info("Killing " + this);
+		isKilled = true;
+		bf.freePage(levelPage);
 	}
 
 	public SkipLevels newInstance(int levels, SkipSpan ss, SkipList sl) {
@@ -150,4 +169,12 @@ public class BSkipLevels extends SkipLevels {
 		if (levels[0] != null)
 			levels[0].blvlck(fix, width + 1);
 	}
+
+	@Override
+	public String toString() {
+		String rv = "BSL height: " + levels.length + " page: " + levelPage + " span: " + bottom;
+		if (isKilled)
+			rv += " KILLED";
+		return rv;
+	}
 }

core/java/src/net/metanotion/io/block/index/BSkipList.java

@@ -44,6 +44,8 @@ import net.metanotion.util.skiplist.*;
  *    first level page (unsigned int)
  *    size (unsigned int)
  *    spans (unsigned int)
+ *
+ * Always fits on one page.
  */
 public class BSkipList extends SkipList {
 	private static final long MAGIC = 0x536b69704c697374l;  // "SkipList"
@@ -138,8 +140,8 @@ public class BSkipList extends SkipList {
 
 	public int maxLevels() {
 		int max = super.maxLevels();
-		int cells = (int) ((BlockFile.PAGESIZE - 8) / 4);
+		int cells = (BlockFile.PAGESIZE - BSkipLevels.HEADER_LEN) / 4;
-		return (max > cells) ? cells : max;
+		return Math.min(cells, max);
 	}
 
 	@Override
@@ -179,9 +181,7 @@ public class BSkipList extends SkipList {
 		BlockFile.log.warn("    firstSpanPage " + this.firstSpanPage);
 		BlockFile.log.warn("    firstLevelPage " + this.firstLevelPage);
 		BlockFile.log.warn("    maxLevels " + this.maxLevels());
-		BlockFile.log.warn("*** printSL() ***");
 		printSL();
-		BlockFile.log.warn("*** print() ***");
 		print();
 		BlockFile.log.warn("*** Lvlck() ***");
 		stack.blvlck(fix, 0);

core/java/src/net/metanotion/io/block/index/BSkipSpan.java

@@ -71,6 +71,7 @@ public class BSkipSpan extends SkipSpan {
 
 	// I2P
 	protected int spanSize;
+	protected boolean isKilled;
 
 	public static void init(BlockFile bf, int page, int spanSize) throws IOException {
 		BlockFile.pageSeek(bf.file, page);
@@ -91,18 +92,27 @@ public class BSkipSpan extends SkipSpan {
 	}
 
 	public void killInstance() {
+		if (isKilled) {
+			BlockFile.log.error("Already killed!! " + this, new Exception());
+			return;
+		}
+		BlockFile.log.info("Killing " + this);
+		isKilled = true;
 		try {
 			int curPage = overflowPage;
-			int next;
+			bf.freePage(page);
 			while(curPage != 0) {
 				BlockFile.pageSeek(bf.file, curPage);
-				bf.file.skipBytes(4);   // skip magic
+				int magic = bf.file.readInt();
-				next = bf.file.readUnsignedInt();
+				if (magic != BlockFile.MAGIC_CONT)
+					throw new IOException("Bad SkipSpan magic number 0x" + Integer.toHexString(magic) + " on page " + curPage);
+				int next = bf.file.readUnsignedInt();
 				bf.freePage(curPage);
 				curPage = next;
 			}
-			bf.freePage(page);
+		} catch (IOException ioe) {
-		} catch (IOException ioe) { throw new RuntimeException("Error freeing database page", ioe); }
+			BlockFile.log.error("Error freeing " + this, ioe);
+		}
 	}
 
 	public void flush() {
@@ -113,12 +123,21 @@ public class BSkipSpan extends SkipSpan {
 	 * I2P - avoid super.flush()
 	 */
 	private void fflush() {
+		if (isKilled) {
+			BlockFile.log.error("Already killed!! " + this, new Exception());
+			return;
+		}
 		try {
 			BlockFile.pageSeek(bf.file, page);
 			bf.file.writeInt(MAGIC);
 			bf.file.writeInt(overflowPage);
-			bf.file.writeInt((prev != null) ? ((BSkipSpan) prev).page : 0);
+			prevPage = (prev != null) ? ((BSkipSpan) prev).page : 0;
-			bf.file.writeInt((next != null) ? ((BSkipSpan) next).page : 0);
+			nextPage = (next != null) ? ((BSkipSpan) next).page : 0;
+			bf.file.writeInt(prevPage);
+			bf.file.writeInt(nextPage);
+			// if keys is null, we are (hopefully) just updating the prev/next pages on an unloaded span
+			if (keys == null)
+				return;
 			bf.file.writeShort((short) keys.length);
 			bf.file.writeShort((short) nKeys);
 
@@ -192,6 +211,8 @@ public class BSkipSpan extends SkipSpan {
 	 * Only read the span headers
 	 */
 	protected static void loadInit(BSkipSpan bss, BlockFile bf, BSkipList bsl, int spanPage, Serializer key, Serializer val) throws IOException {
+		if (bss.isKilled)
+			BlockFile.log.error("Already killed!! " + bss, new Exception());
 		bss.bf = bf;
 		bss.page = spanPage;
 		bss.keySer = key;
@@ -209,6 +230,8 @@ public class BSkipSpan extends SkipSpan {
 		bss.nextPage = bf.file.readUnsignedInt();
 		bss.spanSize = bf.file.readUnsignedShort();
 		bss.nKeys = bf.file.readUnsignedShort();
+		if(bss.spanSize < 1 || bss.spanSize > SkipSpan.MAX_SIZE || bss.nKeys > bss.spanSize)
+			throw new IOException("Invalid span size " + bss.nKeys + " / "+  bss.spanSize);
 	}
 
 	/**
@@ -225,6 +248,8 @@ public class BSkipSpan extends SkipSpan {
 	 * @param flushOnError set to false if you are going to flush anyway
 	 */
 	protected void loadData(boolean flushOnError) throws IOException {
+		if (isKilled)
+			BlockFile.log.error("Already killed!! " + this, new Exception());
 		this.keys = new Comparable[this.spanSize];
 		this.vals = new Object[this.spanSize];
 
@@ -317,4 +342,12 @@ public class BSkipSpan extends SkipSpan {
 			np = bss.prevPage;
 		}
 	}
+
+	@Override
+	public String toString() {
+		String rv = "BSS page: " + page + " key: \"" + firstKey() + '"';
+		if (isKilled)
+			rv += " KILLED";
+		return rv;
+	}
 }

core/java/src/net/metanotion/io/block/index/IBSkipSpan.java

@@ -79,14 +79,19 @@ public class IBSkipSpan extends BSkipSpan {
 	@Override
 	public void flush() {
 		super.flush();
-		if (nKeys > 0)
+		if (nKeys <= 0)
-			this.firstKey = keys[0];
-		else
 			this.firstKey = null;
-		this.keys = null;
+		if (keys != null) {
-		this.vals = null;
+			if (nKeys > 0)
-		if (BlockFile.log.shouldLog(Log.DEBUG))
+				this.firstKey = keys[0];
-			BlockFile.log.debug("Flushed data for page " + this.page + " containing " + this.nKeys + '/' + this.spanSize);
+			this.keys = null;
+			this.vals = null;
+			if (BlockFile.log.shouldLog(Log.DEBUG))
+				BlockFile.log.debug("Flushed data for page " + this.page + " containing " + this.nKeys + '/' + this.spanSize);
+		} else if (BlockFile.log.shouldLog(Log.DEBUG)) {
+			// if keys is null, we are (hopefully) just updating the prev/next pages on an unloaded span
+			BlockFile.log.debug("Flushed pointers for for unloaded page " + this.page + " containing " + this.nKeys + '/' + this.spanSize);
+		}
 	}
 
 	/**
@@ -133,6 +138,8 @@ public class IBSkipSpan extends BSkipSpan {
 	 * Seek past the span header
 	 */
 	private void seekData() throws IOException {
+		if (isKilled)
+			throw new IOException("Already killed! " + this);
 		BlockFile.pageSeek(this.bf.file, this.page);
 		int magic = bf.file.readInt();
 		if (magic != MAGIC)
@@ -333,10 +340,20 @@ public class IBSkipSpan extends BSkipSpan {
 	 */
 	@Override
 	public Object[] remove(Comparable key, SkipList sl) {
+		if (BlockFile.log.shouldLog(Log.DEBUG))
+			BlockFile.log.debug("Remove " + key + " in " + this);
+		if (nKeys <= 0)
+			return null;
 		try {
 			seekAndLoadData();
+			if (this.nKeys == 1 && this.prev == null && this.next != null && this.next.keys == null) {
+				// fix for NPE in SkipSpan if next is not loaded
+				if (BlockFile.log.shouldLog(Log.INFO))
+					BlockFile.log.info("Loading next data for remove");
+				((IBSkipSpan)this.next).seekAndLoadData();
+			}
 		} catch (IOException ioe) {
-			throw new RuntimeException("Error reading database", ioe);
+			throw new RuntimeException("Error reading database attempting to remove " + key, ioe);
 		}
 		Object[] rv = super.remove(key, sl);
 		// flush() nulls out the data

core/java/src/net/metanotion/util/skiplist/SkipLevels.java

@@ -30,7 +30,12 @@ package net.metanotion.util.skiplist;
 
 import net.metanotion.io.block.BlockFile;
 
+import net.i2p.util.Log;
+
 public class SkipLevels {
+	/** We can't have more than 2**32 pages */
+	public static final int MAX_SIZE = 32;
+
 	/*
 	 *	"Next" pointers
 	 *	The highest indexed level is the "highest" level in the list.
@@ -46,25 +51,40 @@ public class SkipLevels {
 	public void flush() { }
 
 	protected SkipLevels() { }
+
+	/*
+	 *  @throws IllegalArgumentException if size too big or too small
+	 */
 	public SkipLevels(int size, SkipSpan span) {
-		if(size < 1) { throw new RuntimeException("Invalid Level Skip size"); }
+		if(size < 1 || size > MAX_SIZE)
+			throw new IllegalArgumentException("Invalid Level Skip size");
 		levels = new SkipLevels[size];
 		bottom = span;
 	}
 
-	public void print() {
+	public String print() {
-		System.out.print("SL:" + key() + "::");
+		StringBuilder buf = new StringBuilder(128);
+		buf.append("SL: ").append(key()).append(" :: ");
 		for(int i=0;i<levels.length;i++) {
+			buf.append(i);
 			if(levels[i] != null) {
-				System.out.print(i + "->" + levels[i].key() + " ");
+				buf.append("->").append(levels[i].key()).append(' ');
 			} else {
-				System.out.print(i + "->() ");
+				buf.append("->() ");
 			}
 		}
-		System.out.print("\n");
+		buf.append('\n');
+		return buf.toString();
+	}
+
+	public String printAll() {
+		StringBuilder buf = new StringBuilder(128);
+		buf.append(print());
 		if(levels[0] != null) {
-			levels[0].print();
+			buf.append('\n');
+			buf.append(levels[0].print());
 		}
+		return buf.toString();
 	}
 
 	public SkipSpan getEnd() {
@@ -104,15 +124,17 @@ public class SkipLevels {
 	public Object[] remove(int start, Comparable key, SkipList sl) {
 		Object[] res = null;
 		SkipLevels slvls = null;
-		for(int i=Math.min(start, levels.length - 1);i>=0;i--) {
+		for(int i = Math.min(start, levels.length - 1); i >= 0; i--) {
 			if(levels[i] != null) {
 				int cmp = levels[i].key().compareTo(key);
 				if((cmp < 0) || ((i==0) && (cmp <= 0)))  {
 					res = levels[i].remove(i, key, sl);
 					if((res != null) && (res[1] != null)) {
 						slvls = (SkipLevels) res[1];
-						if(levels.length >= slvls.levels.length) { res[1] = null; }
+						if(levels.length >= slvls.levels.length) {
-						for(int j=0;j<(Math.min(slvls.levels.length,levels.length));j++) {
+							res[1] = null;
+						}
+						for(int j = 0 ; j < Math.min(slvls.levels.length, levels.length); j++) {
 							if(levels[j] == slvls) {
 								levels[j] = slvls.levels[j];
 							}
@@ -128,6 +150,41 @@ public class SkipLevels {
 			if(res[1] == bottom) {
 				res[1] = this;
 			} else {
+				// Special handling required if we are the head SkipLevels to fix up our level pointers
+				// if the returned SkipSpan was already copied to us
+				boolean isFirst = sl.first == bottom;
+				if (isFirst && levels[0] != null) {
+					SkipSpan ssres = (SkipSpan)res[1];
+					if (bottom.firstKey().equals(ssres.firstKey())) {
+						// bottom copied the next span to itself
+						if (BlockFile.log.shouldLog(Log.INFO)) {
+							BlockFile.log.info("First Level, bottom.remove() copied and did not return itself!!!! in remove " + key);
+							BlockFile.log.info("Us:     " + print());
+							BlockFile.log.info("next:   " + levels[0].print());
+							BlockFile.log.info("ssres.firstKey():   " + ssres.firstKey());
+							BlockFile.log.info("ssres.keys[0]:   " + ssres.keys[0]);
+							BlockFile.log.info("FIXUP TIME");
+						}
+						
+						SkipLevels replace = levels[0];
+						for (int i = 0; i < levels.length; i++) {
+							if (levels[i] == null)
+								break;
+							if (i >= replace.levels.length)
+								break;
+							if (levels[i].key().equals(replace.key())) {
+								if (BlockFile.log.shouldLog(Log.INFO))
+							        	BlockFile.log.info("equal level " + i);
+								levels[i] = replace.levels[i];
+							} else if (BlockFile.log.shouldLog(Log.INFO)) {
+								BlockFile.log.info("not equal level " + i + ' ' + levels[i].key());
+							}
+						}
+						if (BlockFile.log.shouldLog(Log.INFO))
+							BlockFile.log.info("new Us: " + print());
+						replace.killInstance();
+					}
+				}
 				res[1] = null;
 			}
 		}

core/java/src/net/metanotion/util/skiplist/SkipList.java

@@ -46,8 +46,13 @@ public class SkipList {
 	public void flush() { }
 	protected SkipList() { }
 
+	/*
+	 *  @param span span size
+	 *  @throws IllegalArgumentException if size too big or too small
+	 */
 	public SkipList(int span) {
-		if(span < 1) { throw new RuntimeException("Span size too small"); }
+		if(span < 1 || span > SkipSpan.MAX_SIZE)
+			throw new IllegalArgumentException("Invalid span size");
 		first = new SkipSpan(span);
 		stack = new SkipLevels(1, first);
 		spans = 1;
@@ -122,14 +127,16 @@ public class SkipList {
 		return null;
 	}
 
+	/** dumps all the skip levels */
 	public void printSL() {
 		System.out.println("List size " + size + " spans " + spans);
-		stack.print();
+		System.out.println(stack.printAll());
 	}
 
+	/** dumps all the data */
 	public void print() {
 		System.out.println("List size " + size + " spans " + spans);
-		first.print();
+		System.out.println(first.print());
 	}
 
 	public Object get(Comparable key) {

core/java/src/net/metanotion/util/skiplist/SkipSpan.java

@@ -28,7 +28,12 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */
 package net.metanotion.util.skiplist;
 
+//import net.metanotion.io.block.BlockFile;
+
 public class SkipSpan {
+	/** This is actually limited by BlockFile.spanSize which is much smaller */
+	public static final int MAX_SIZE = 256;
+
 	public int nKeys = 0;
 	public Comparable[] keys;
 	public Object[] vals;
@@ -39,19 +44,28 @@ public class SkipSpan {
 	public void flush() { }
 
 	protected SkipSpan() { }
+
+	/*
+	 *  @throws IllegalArgumentException if size too big or too small
+	 */
 	public SkipSpan(int size) {
+		if(size < 1 || size > MAX_SIZE)
+			throw new IllegalArgumentException("Invalid span size " + size);
 		keys = new Comparable[size];
 		vals = new Object[size];
 	}
 
-	public void print() {
+	/** dumps all the data from here to the end */
-		System.out.println("Span containing " + nKeys + " keys");
+	public String print() {
+		StringBuilder buf = new StringBuilder(1024);
+		buf.append("Span with ").append(nKeys).append(" keys\n");
 		if (nKeys > 0 && keys != null && vals != null) {
 			for(int i=0;i<nKeys;i++) {
-				System.out.println("\t" + keys[i] + " => " + vals[i]);
+				buf.append('\t').append(keys[i]).append(" => ").append(vals[i]).append('\n');
 			}
 		}
-		if(next != null) { next.print(); }
+		if (next != null) { buf.append(next.print()); }
+		return buf.toString();
 	}
 
 	private int binarySearch(Comparable key) {
@@ -247,18 +261,26 @@ public class SkipSpan {
 			if(sl.spans > 1) { sl.spans--; }
 			if((this.prev == null) && (this.next != null)) {
 				res[1] = this.next;
-				// We're the first node in the list...
+				// We're the first node in the list... copy the next node over and kill it. See also bottom of SkipLevels.java
 				for(int i=0;i<next.nKeys;i++) {
 					keys[i] = next.keys[i];
 					vals[i] = next.vals[i];
 				}
+
 				nKeys = next.nKeys;
+				//BlockFile.log.error("Killing next span " + next + ") and copying to this span " + this + " in remove of " + key);
+				// Make us point to next.next and him point back to us
 				SkipSpan nn = next.next;
 				next.killInstance();
-				this.flush();
+				if (nn != null) {
+					nn.prev = this;
+					nn.flush();
+				}
 				this.next = nn;
+				this.flush();
 			} else {
-				res[1] = this;
+				// Normal situation. We are now empty, kill ourselves
+				//BlockFile.log.error("Killing this span " + this + ", prev " + this.prev + ", next " + this.next);
 				if(this.prev != null) {
 					this.prev.next = this.next;
 					this.prev.flush();
@@ -266,11 +288,20 @@ public class SkipSpan {
 				if(this.next != null) {
 					this.next.prev = this.prev;
 					this.next.flush();
+					this.next = null;
+				}
+				if (this.prev != null) {
+					// Kill ourselves
+					this.prev = null;
+					this.killInstance();
+					res[1] = this;
+				} else {
+					// Never kill first span
+					//BlockFile.log.error("Not killing First span, now empty!!!!!!!!!!!!!!!!!!");
+					this.flush();
+					res[1] = null;
 				}
-				this.next = null;
-				this.prev = null;
 				nKeys = 0;
-				this.killInstance();
 			}
 		} else {
 			pushTogether(loc);