From 2eaf8f7ce0ced2b5660f41a169447a8520bcc943 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Thu, 29 May 2014 08:02:15 -0600 Subject: [PATCH] Add freelist assertion on every free(). This commit performs a check on the freelist pages to ensure that a double free can never happen. --- db_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++ freelist.go | 46 ++++++++++++++++++++-------------------------- tx_test.go | 25 ------------------------- 3 files changed, 66 insertions(+), 51 deletions(-) diff --git a/db_test.go b/db_test.go index e1c4aaa..691c8fb 100644 --- a/db_test.go +++ b/db_test.go @@ -335,6 +335,52 @@ func TestMeta_validate_version(t *testing.T) { assert.Equal(t, m.validate(), ErrVersionMismatch) } +// Ensure that a DB in strict mode will fail when corrupted. +func TestDB_StrictMode(t *testing.T) { + var msg string + func() { + defer func() { + msg = fmt.Sprintf("%s", recover()) + }() + + withOpenDB(func(db *DB, path string) { + db.StrictMode = true + db.Update(func(tx *Tx) error { + tx.CreateBucket([]byte("foo")) + + // Corrupt the DB by extending the high water mark. + tx.meta.pgid++ + + return nil + }) + }) + }() + + assert.Equal(t, "check fail: page 4: unreachable unfreed", msg) +} + +// Ensure that a double freeing a page will result in a panic. +func TestDB_DoubleFree(t *testing.T) { + var msg string + func() { + defer func() { + msg = fmt.Sprintf("%s", recover()) + }() + withOpenDB(func(db *DB, path string) { + db.Update(func(tx *Tx) error { + tx.CreateBucket([]byte("foo")) + + // Corrupt the DB by adding a page to the freelist. + db.freelist.free(0, tx.page(3)) + + return nil + }) + }) + }() + + assert.Equal(t, "tx 2: page 3 already freed in tx 0", msg) +} + func ExampleDB_Update() { // Open the database. db, _ := Open(tempfile(), 0666) diff --git a/freelist.go b/freelist.go index 149e595..a236079 100644 --- a/freelist.go +++ b/freelist.go @@ -1,6 +1,7 @@ package bolt import ( + "fmt" "sort" "unsafe" ) @@ -67,15 +68,31 @@ func (f *freelist) allocate(n int) pgid { } // free releases a page and its overflow for a given transaction id. +// If the page is already free then a panic will occur. func (f *freelist) free(txid txid, p *page) { - var ids = f.pending[txid] _assert(p.id > 1, "cannot free page 0 or 1: %d", p.id) + + // Verify that page is not already free. + minid, maxid := p.id, p.id+pgid(p.overflow) + for _, id := range f.ids { + if id >= minid && id <= maxid { + panic(fmt.Sprintf("page %d already freed in tx", id)) + } + } + for ptxid, m := range f.pending { + for _, id := range m { + if id >= minid && id <= maxid { + panic(fmt.Sprintf("tx %d: page %d already freed in tx %d", txid, id, ptxid)) + } + } + } + + // Free page and all its overflow pages. + var ids = f.pending[txid] for i := 0; i < int(p.overflow+1); i++ { ids = append(ids, p.id+pgid(i)) } f.pending[txid] = ids - - // DEBUG ONLY: f.check() } // release moves all page ids for a transaction id (or older) to the freelist. @@ -123,26 +140,3 @@ func (f *freelist) write(p *page) { p.count = uint16(len(ids)) copy(((*[maxAllocSize]pgid)(unsafe.Pointer(&p.ptr)))[:], ids) } - -// check verifies there are no double free pages. -// This is slow so it should only be used while debugging. -// If errors are found then a panic invoked. -/* -func (f *freelist) check() { - var lookup = make(map[pgid]txid) - for _, id := range f.ids { - if _, ok := lookup[id]; ok { - panic(fmt.Sprintf("page %d already freed", id)) - } - lookup[id] = 0 - } - for txid, m := range f.pending { - for _, id := range m { - if _, ok := lookup[id]; ok { - panic(fmt.Sprintf("tx %d: page %d already freed in tx %d", txid, id, lookup[id])) - } - lookup[id] = txid - } - } -} -*/ diff --git a/tx_test.go b/tx_test.go index 5cbe20f..2fe4723 100644 --- a/tx_test.go +++ b/tx_test.go @@ -288,31 +288,6 @@ func TestTx_OnCommit_Rollback(t *testing.T) { assert.Equal(t, 0, x) } -// Ensure that a Tx in strict mode will fail when corrupted. -func TestTx_Check_Corrupt(t *testing.T) { - var msg string - func() { - defer func() { - msg = fmt.Sprintf("%s", recover()) - }() - - withOpenDB(func(db *DB, path string) { - db.StrictMode = true - db.Update(func(tx *Tx) error { - tx.CreateBucket([]byte("foo")) - - // Corrupt the DB by adding a page to the freelist. - warn("---") - db.freelist.free(0, tx.page(3)) - - return nil - }) - }) - }() - - assert.Equal(t, "check fail: page 3: already freed", msg) -} - // Ensure that the database can be copied to a file path. func TestTx_CopyFile(t *testing.T) { withOpenDB(func(db *DB, path string) {