From d244f0189801fac317806e5908a47604b333ebd7 Mon Sep 17 00:00:00 2001 From: WizardCXY Date: Wed, 13 Mar 2019 11:34:15 +0800 Subject: [PATCH] fix rollback panic bug --- freelist.go | 24 ++++++++++++++++++++++++ tx.go | 8 +++++++- tx_test.go | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/freelist.go b/freelist.go index 93fd85d50..8ce765a7c 100644 --- a/freelist.go +++ b/freelist.go @@ -349,6 +349,30 @@ func (f *freelist) reload(p *page) { f.readIDs(a) } +// noSyncReload reads the freelist from pgids and filters out pending items. +func (f *freelist) noSyncReload(pgids []pgid) { + // Build a cache of only pending pages. + pcache := make(map[pgid]bool) + for _, txp := range f.pending { + for _, pendingID := range txp.ids { + pcache[pendingID] = true + } + } + + // Check each page in the freelist and build a new available freelist + // with any pages not in the pending lists. + var a []pgid + for _, id := range pgids { + if !pcache[id] { + a = append(a, id) + } else { + panic("should not happen") + } + } + + f.readIDs(a) +} + // reindex rebuilds the free cache based on available and pending free lists. func (f *freelist) reindex() { ids := f.getFreePageIDs() diff --git a/tx.go b/tx.go index f50864142..2b8c14dbf 100644 --- a/tx.go +++ b/tx.go @@ -264,7 +264,13 @@ func (tx *Tx) rollback() { } if tx.writable { tx.db.freelist.rollback(tx.meta.txid) - tx.db.freelist.reload(tx.db.page(tx.db.meta().freelist)) + if !tx.db.hasSyncedFreelist() { + // Reconstruct free page list by scanning the DB. + tx.db.freelist.noSyncReload(tx.db.freepages()) + } else { + // Read free page list from freelist page. + tx.db.freelist.reload(tx.db.page(tx.db.meta().freelist)) + } } tx.close() } diff --git a/tx_test.go b/tx_test.go index 4760c7ad3..e804a720e 100644 --- a/tx_test.go +++ b/tx_test.go @@ -654,6 +654,56 @@ func TestTx_CopyFile_Error_Normal(t *testing.T) { } } +func TestRollbackWithNoFreelistSync(t *testing.T) { + for _, isSyncFreelist := range []bool{false, true} { + // Open the database. + db, err := bolt.Open(tempfile(), 0666, nil) + if err != nil { + log.Fatal(err) + } + defer os.Remove(db.Path()) + db.NoFreelistSync = isSyncFreelist + + tx, err := db.Begin(true) + if err != nil { + t.Fatalf("Error starting tx: %v", err) + } + bucket := []byte("mybucket") + if _, err := tx.CreateBucket(bucket); err != nil { + t.Fatalf("Error creating bucket: %v", err) + } + if err := tx.Commit(); err != nil { + t.Fatalf("Error on commit: %v", err) + } + + tx, err = db.Begin(true) + if err != nil { + t.Fatalf("Error starting tx: %v", err) + } + b := tx.Bucket(bucket) + if err := b.Put([]byte("k"), []byte("v")); err != nil { + t.Fatalf("Error on put: %v", err) + } + // Imagine there is an error and tx needs to be rolled-back + if err := tx.Rollback(); err != nil { + t.Fatalf("Error on rollback: %v", err) + } + + tx, err = db.Begin(false) + if err != nil { + t.Fatalf("Error starting tx: %v", err) + } + b = tx.Bucket(bucket) + if v := b.Get([]byte("k")); v != nil { + t.Fatalf("Value for k should not have been stored") + } + if err := tx.Rollback(); err != nil { + t.Fatalf("Error on rollback: %v", err) + } + + } +} + // TestTx_releaseRange ensures db.freePages handles page releases // correctly when there are transaction that are no longer reachable // via any read/write transactions and are "between" ongoing read