From 8be8127351396593af941395f59c42e136f3e698 Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Sun, 24 Sep 2017 20:00:42 -0600 Subject: [PATCH 1/3] db: fix MemDB.Close Fixes https://github.com/tendermint/tmlibs/issues/55 MemDB previously mistakenly set the actual DB pointer to nil although that side effect is not visible to the outside world since p is an identifier within the scope of just that function call. However, @melekes and I had a discussion in which we came to the conclusion that Close for an in-memory DB should instead be a noop and not cause any data loss. See the discussion on https://github.com/tendermint/tmlibs/pull/56. --- db/mem_db.go | 8 +++++--- db/mem_db_test.go | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/db/mem_db.go b/db/mem_db.go index db40227e..58e74895 100644 --- a/db/mem_db.go +++ b/db/mem_db.go @@ -52,9 +52,11 @@ func (db *MemDB) DeleteSync(key []byte) { } func (db *MemDB) Close() { - db.mtx.Lock() - defer db.mtx.Unlock() - db = nil + // Close is a noop since for an in-memory + // database, we don't have a destination + // to flush contents to nor do we want + // any data loss on invoking Close() + // See the discussion in https://github.com/tendermint/tmlibs/pull/56 } func (db *MemDB) Print() { diff --git a/db/mem_db_test.go b/db/mem_db_test.go index a76e10dc..503e361f 100644 --- a/db/mem_db_test.go +++ b/db/mem_db_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestMemDbIterator(t *testing.T) { @@ -26,3 +27,22 @@ func TestMemDbIterator(t *testing.T) { } assert.Equal(t, i, len(db.db), "iterator didnt cover whole db") } + +func TestMemDBClose(t *testing.T) { + db := NewMemDB() + copyDB := func(orig map[string][]byte) map[string][]byte { + copy := make(map[string][]byte) + for k, v := range orig { + copy[k] = v + } + return copy + } + k, v := []byte("foo"), []byte("bar") + db.Set(k, v) + require.Equal(t, db.Get(k), v, "expecting a successful get") + copyBefore := copyDB(db.db) + db.Close() + require.Equal(t, db.Get(k), v, "Close is a noop, expecting a successful get") + copyAfter := copyDB(db.db) + require.Equal(t, copyBefore, copyAfter, "Close is a noop and shouldn't modify any internal data") +} From 0948343a6fd4efce07894c312d8e96c5f277c608 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 2 Oct 2017 14:17:16 -0400 Subject: [PATCH 2/3] autofile: ensure file is open in Sync --- autofile/autofile.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/autofile/autofile.go b/autofile/autofile.go index 5d6bc726..05fb0d67 100644 --- a/autofile/autofile.go +++ b/autofile/autofile.go @@ -103,6 +103,11 @@ func (af *AutoFile) Sync() error { af.mtx.Lock() defer af.mtx.Unlock() + if af.file == nil { + if err := af.openFile(); err != nil { + return err + } + } return af.file.Sync() } From e9c83b30058f60c8bf6810d52ab491712b9cedf7 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 2 Oct 2017 23:26:45 -0400 Subject: [PATCH 3/3] version and changelog --- CHANGELOG.md | 8 ++++++++ version/version.go | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f5bd59c..e36a02d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## 0.3.2 (October 2, 2017) + +BUG FIXES: + +- [autofile] fix AutoFile.Sync() to open file if it's been closed +- [db] fix MemDb.Close() to not empty the database (ie. its just a noop) + + ## 0.3.1 (September 22, 2017) BUG FIXES: diff --git a/version/version.go b/version/version.go index 6e030624..77580b5a 100644 --- a/version/version.go +++ b/version/version.go @@ -1,3 +1,3 @@ package version -const Version = "0.3.1" +const Version = "0.3.2"