npe fix and defensive code

db/btree.cpp

@@ -866,34 +866,48 @@ void BtreeCursor::noteLocation() {
 	}
 }
 
-int clctr = 0;
+/* Since the last noteLocation(), our key may have moved around, and that old cached 
+   information may thus be stale and wrong (although often it is right).  We check 
+   that here; if we have moved, we have to search back for where we were at.
 
-/* see if things moved around (deletes, splits, inserts) */
+   i.e., after operations on the index, the BtreeCursor's cached location info may 
+   be invalid.  This function ensures validity, so you should call it before using 
+   the cursor if other writers have used the database since the last noteLocation
+   call.
+*/
 void BtreeCursor::checkLocation() { 
-	try {
+	{
 		if( eof() ) 
 			return;
-		BtreeBucket *b = bucket.btree();
+		BtreeBucket *b = bucket.btree(); 
+
+		assert( !keyAtKeyOfs.isEmpty() ); 
+
+		// Note keyAt() returns an empty JSObj if keyOfs is now out of range, 
+		// which is possible as keys may have been deleted.
 		if( b->keyAt(keyOfs).woEqual(keyAtKeyOfs) &&
 			b->k(keyOfs).recordLoc == locAtKeyOfs ) { 
-			if( !b->k(keyOfs).isUsed() )
-				checkUnused();
-			return;
+				if( !b->k(keyOfs).isUsed() ) {
+					/* we were deleted but still exist as an unused 
+					   marker key. advance. 
+					*/
+					checkUnused();
+				}
+				return;
 		}
 	}
-	catch( AssertionException ) { 
-		cout << "Caught exception in checkLocation(), that's maybe ok" << endl;
-	}
+
+	/* normally we don't get to here.  when we do, old position is no longer
+ 	   valid and we must refind where we left off (which is expensive)
+	*/
 
 	bool found;
 	DiskLoc bold = bucket;
 
-/* TODO: Switch to keep indexdetails and do idx.head! */
-	/* didn't find, check from the top */
+	/* TODO: Switch to keep indexdetails and do idx.head! */
 	DiskLoc head = bold.btree()->getHead(bold);
 	bucket = head.btree()->locate(head, keyAtKeyOfs, keyOfs, found, locAtKeyOfs, direction);
-	if( clctr++ % 128 == 0 )
-		cout << "  key seems to have moved in the index, refinding. found:" << found << endl;
+	RARELY cout << "  key seems to have moved in the index, refinding. found:" << found << endl;
 	if( found )
 		checkUnused();
 }

db/btree.h

@@ -17,8 +17,16 @@ struct _KeyNode {
 	unsigned short _kdo;
 	void setKeyDataOfs(short s) { _kdo = s; assert(s>=0); }
 	void setKeyDataOfsSavingUse(short s) { _kdo = s; assert(s>=0); }
-	void setUnused() { recordLoc.GETOFS() |= 1; }
-//	void setUsed() { _kdo &= 0x7fff; }
+	void setUnused() { 
+		/* setting ofs to 1 is the sentinel.  setInvalid sets the 
+		   fileno, that is defensive code. 
+		*/
+		recordLoc.setInvalid();
+		recordLoc.GETOFS() = 1; 
+	}
+	/* & 1 for backward compatibility.  can be made "== 1" later when we increment
+	   the db data version 
+    */
 	int isUnused() { return (recordLoc.getOfs() & 1); }
 	int isUsed() { return !isUnused(); }
 };

db/jsobj.h

@@ -170,16 +170,7 @@ class JSObj {
 	friend class JSElemIter;
 	class Details {
 	public:
-        ~Details() {
-            
-            assert(refCount <= 0);
-            
-            // note that the assert above protects owned() from lying to us.  If the assert() goes, fix this
-            if (owned()) {
-                free((void *)_objdata);
-            }
-            _objdata = 0;
-        }
+		~Details() { _objdata = 0; }
 		const char *_objdata;
 		int _objsize;
 		int refCount; // -1 == don't free
@@ -233,8 +224,8 @@ public:
 	JSObj extractFields(JSObj pattern, JSObjBuilder& b);
 
 	const char *objdata() const { return details->_objdata; }
-	int objsize() const { return details->_objsize; } // includes the embedded size field
-	bool isEmpty() const { return details == 0 || objsize() <= 5; }
+	int objsize() const { return details ? details->_objsize : 0; } // includes the embedded size field
+	bool isEmpty() const { return objsize() <= 5; }
 
 	/* this is broken if elements aren't in the same order. */
 	bool operator<(const JSObj& r) const { return woCompare(r) < 0; }
@@ -244,7 +235,9 @@ public:
 	*/
 	int woCompare(const JSObj& r) const;
 	bool woEqual(const JSObj& r) const { 
-		return objsize()==r.objsize() && memcmp(objdata(),r.objdata(),objsize())==0;
+		int os = objsize();
+		return os == r.objsize() &&
+			(os == 0 || memcmp(objdata(),r.objdata(),os)==0);
 	}
 	bool operator==(const JSObj& r) const { 
 		return this->woEqual(r);

db/pdfile.cpp

@@ -717,8 +717,6 @@ void DataFileMgr::deleteRecord(const char *ns, Record *todelete, const DiskLoc&
 {
 	dassert( todelete == dl.rec() );
 
-	int tempextofs = todelete->extentOfs;
-
 	NamespaceDetails* d = nsdetails(ns);
 	if( d->capped && !cappedOK ) { 
 		cout << "failing remove on a capped ns " << ns << endl;

db/query.cpp

@@ -150,7 +150,7 @@ auto_ptr<Cursor> getIndexCursor(const char *ns, JSObj& query, JSObj& order, bool
 	}
 
 fail:
-	DEV cout << "getIndexCursor fail" << endl;
+	DEV cout << "getIndexCursor fail " << ns << '\n';
 	return auto_ptr<Cursor>();
 }