From f46ed4aac888ce619b4b2f94fb6998fb5da76a1b Mon Sep 17 00:00:00 2001 From: Yawning Angel Date: Fri, 7 Jun 2019 07:22:02 +0000 Subject: [PATCH 1/2] libs/db: Fix the BoltDB Batch.Delete `Delete` should queue a delete operation that targets the entire database, and not just keys that happen to be in the current batch. --- CHANGELOG_PENDING.md | 1 + libs/db/boltdb.go | 44 +++++++++++++++++--------------------------- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 8a8ad2795..286c1cb02 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -28,3 +28,4 @@ * [rpc] [\#3686](https://github.com/tendermint/tendermint/pull/3686) `HTTPClient#Call` returns wrapped errors, so a caller could use `errors.Cause` to retrieve an error code. ### BUG FIXES: +- [libs/db] Fixed the BoltDB backend's Batch.Delete implementation (@Yawning) diff --git a/libs/db/boltdb.go b/libs/db/boltdb.go index 6bf474c86..6b44c5560 100644 --- a/libs/db/boltdb.go +++ b/libs/db/boltdb.go @@ -149,55 +149,45 @@ func (bdb *BoltDB) Stats() map[string]string { // boltDBBatch stores key values in sync.Map and dumps them to the underlying // DB upon Write call. type boltDBBatch struct { - buffer []struct { - k []byte - v []byte - } - db *BoltDB + db *BoltDB + ops []operation } // NewBatch returns a new batch. func (bdb *BoltDB) NewBatch() Batch { return &boltDBBatch{ - buffer: make([]struct { - k []byte - v []byte - }, 0), - db: bdb, + ops: nil, + db: bdb, } } // It is safe to modify the contents of the argument after Set returns but not // before. func (bdb *boltDBBatch) Set(key, value []byte) { - bdb.buffer = append(bdb.buffer, struct { - k []byte - v []byte - }{ - key, value, - }) + bdb.ops = append(bdb.ops, operation{opTypeSet, key, value}) } // It is safe to modify the contents of the argument after Delete returns but // not before. func (bdb *boltDBBatch) Delete(key []byte) { - for i, elem := range bdb.buffer { - if bytes.Equal(elem.k, key) { - // delete without preserving order - bdb.buffer[i] = bdb.buffer[len(bdb.buffer)-1] - bdb.buffer = bdb.buffer[:len(bdb.buffer)-1] - return - } - } + bdb.ops = append(bdb.ops, operation{opTypeDelete, key, nil}) } // NOTE: the operation is synchronous (see BoltDB for reasons) func (bdb *boltDBBatch) Write() { err := bdb.db.db.Batch(func(tx *bbolt.Tx) error { b := tx.Bucket(bucket) - for _, elem := range bdb.buffer { - if putErr := b.Put(elem.k, elem.v); putErr != nil { - return putErr + for _, op := range bdb.ops { + key := nonEmptyKey(nonNilBytes(op.key)) + switch op.opType { + case opTypeSet: + if putErr := b.Put(key, op.value); putErr != nil { + return putErr + } + case opTypeDelete: + if delErr := b.Delete(key); delErr != nil { + return delErr + } } } return nil From 319ecb3005fe319369f3be91db0fc8605ee2706b Mon Sep 17 00:00:00 2001 From: Yawning Angel Date: Fri, 7 Jun 2019 07:26:47 +0000 Subject: [PATCH 2/2] libs/db: Fix the BoltDB Get and Iterator BoltDB's accessors will return slices that are only valid for the lifetime of the transaction. This adds copies where required to prevent hard to debug crashes (among other things). --- CHANGELOG_PENDING.md | 1 + libs/db/boltdb.go | 12 +++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 286c1cb02..f96956e72 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -29,3 +29,4 @@ ### BUG FIXES: - [libs/db] Fixed the BoltDB backend's Batch.Delete implementation (@Yawning) +- [libs/db] Fixed the BoltDB backend's Get and Iterator implementation (@Yawning) diff --git a/libs/db/boltdb.go b/libs/db/boltdb.go index 6b44c5560..30501dd82 100644 --- a/libs/db/boltdb.go +++ b/libs/db/boltdb.go @@ -66,7 +66,9 @@ func (bdb *BoltDB) Get(key []byte) (value []byte) { key = nonEmptyKey(nonNilBytes(key)) err := bdb.db.View(func(tx *bbolt.Tx) error { b := tx.Bucket(bucket) - value = b.Get(key) + if v := b.Get(key); v != nil { + value = append([]byte{}, v...) + } return nil }) if err != nil { @@ -312,12 +314,16 @@ func (itr *boltDBIterator) Next() { func (itr *boltDBIterator) Key() []byte { itr.assertIsValid() - return itr.currentKey + return append([]byte{}, itr.currentKey...) } func (itr *boltDBIterator) Value() []byte { itr.assertIsValid() - return itr.currentValue + var value []byte + if itr.currentValue != nil { + value = append([]byte{}, itr.currentValue...) + } + return value } func (itr *boltDBIterator) Close() {