Merge pull request #52 from jpbetz/fix-readonly-check

tx: load freelist on Check() [continuation of PR#49]

db.go

@@ -116,9 +116,11 @@ type DB struct {
 	opened   bool
 	rwtx     *Tx
 	txs      []*Tx
-	freelist *freelist
 	stats    Stats
 
+	freelist     *freelist
+	freelistLoad sync.Once
+
 	pagePool sync.Pool
 
 	batchMu sync.Mutex
@@ -157,8 +159,9 @@ func (db *DB) String() string {
 // If the file does not exist then it will be created automatically.
 // Passing in nil options will cause Bolt to open the database with the default options.
 func Open(path string, mode os.FileMode, options *Options) (*DB, error) {
-	var db = &DB{opened: true}
-
+	db := &DB{
+		opened: true,
+	}
 	// Set default options if no options are provided.
 	if options == nil {
 		options = DefaultOptions
@@ -254,20 +257,11 @@ func Open(path string, mode os.FileMode, options *Options) (*DB, error) {
 		return db, nil
 	}
 
-	db.freelist = newFreelist()
-	noFreeList := db.meta().freelist == pgidNoFreelist
-	if noFreeList {
-		// Reconstruct free list by scanning the DB.
-		db.freelist.readIDs(db.freepages())
-	} else {
-		// Read free list from freelist page.
-		db.freelist.read(db.page(db.meta().freelist))
-	}
-	db.stats.FreePageN = len(db.freelist.ids)
+	db.loadFreelist()
 
 	// Flush freelist when transitioning from no sync to sync so
 	// NoFreelistSync unaware boltdb can open the db later.
-	if !db.NoFreelistSync && noFreeList {
+	if !db.NoFreelistSync && !db.hasSyncedFreelist() {
 		tx, err := db.Begin(true)
 		if tx != nil {
 			err = tx.Commit()
@@ -282,6 +276,27 @@ func Open(path string, mode os.FileMode, options *Options) (*DB, error) {
 	return db, nil
 }
 
+// loadFreelist reads the freelist if it is synced, or reconstructs it
+// by scanning the DB if it is not synced. It assumes there are no
+// concurrent accesses being made to the freelist.
+func (db *DB) loadFreelist() {
+	db.freelistLoad.Do(func() {
+		db.freelist = newFreelist()
+		if !db.hasSyncedFreelist() {
+			// Reconstruct free list by scanning the DB.
+			db.freelist.readIDs(db.freepages())
+		} else {
+			// Read free list from freelist page.
+			db.freelist.read(db.page(db.meta().freelist))
+		}
+		db.stats.FreePageN = len(db.freelist.ids)
+	})
+}
+
+func (db *DB) hasSyncedFreelist() bool {
+	return db.meta().freelist != pgidNoFreelist
+}
+
 // mmap opens the underlying memory-mapped file and initializes the meta references.
 // minsz is the minimum size that the new mmap can be.
 func (db *DB) mmap(minsz int) error {

tx.go

@@ -395,6 +395,9 @@ func (tx *Tx) Check() <-chan error {
 }
 
 func (tx *Tx) check(ch chan error) {
+	// Force loading free list if opened in ReadOnly mode.
+	tx.db.loadFreelist()
+
 	// Check if any pages are double freed.
 	freed := make(map[pgid]bool)
 	all := make([]pgid, tx.db.freelist.count())

tx_test.go

@@ -11,6 +11,54 @@ import (
 	"github.com/coreos/bbolt"
 )
 
+// TestTx_Check_ReadOnly tests consistency checking on a ReadOnly database.
+func TestTx_Check_ReadOnly(t *testing.T) {
+	db := MustOpenDB()
+	defer db.Close()
+	if err := db.Update(func(tx *bolt.Tx) error {
+		b, err := tx.CreateBucket([]byte("widgets"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if err := b.Put([]byte("foo"), []byte("bar")); err != nil {
+			t.Fatal(err)
+		}
+		return nil
+	}); err != nil {
+		t.Fatal(err)
+	}
+	if err := db.DB.Close(); err != nil {
+		t.Fatal(err)
+	}
+
+	readOnlyDB, err := bolt.Open(db.f, 0666, &bolt.Options{ReadOnly: true})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer readOnlyDB.Close()
+
+	tx, err := readOnlyDB.Begin(false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// ReadOnly DB will load freelist on Check call.
+	numChecks := 2
+	errc := make(chan error, numChecks)
+	check := func() {
+		err, _ := <-tx.Check()
+		errc <- err
+	}
+	// Ensure the freelist is not reloaded and does not race.
+	for i := 0; i < numChecks; i++ {
+		go check()
+	}
+	for i := 0; i < numChecks; i++ {
+		if err := <-errc; err != nil {
+			t.Fatal(err)
+		}
+	}
+}
+
 // Ensure that committing a closed transaction returns an error.
 func TestTx_Commit_ErrTxClosed(t *testing.T) {
 	db := MustOpenDB()