Problem: memiavl is not protected against concurrent writing (#1100)

* Problem: memiavl is not protected against concurrent writing

Solution:
- introduce exclusive file lock.
- introduce read-only mode which can work concurrently.

* Update CHANGELOG.md

Signed-off-by: yihuang <huang@crypto.com>

* cleanup

* fix same process lock

* file permission

* Update memiavl/db.go

Co-authored-by: mmsqe <mavis@crypto.com>
Signed-off-by: yihuang <huang@crypto.com>

* fix lint

* fix unit test

* cross-platform

* absolute path

---------

Signed-off-by: yihuang <huang@crypto.com>
Co-authored-by: mmsqe <mavis@crypto.com>

CHANGELOG.md

@@ -38,6 +38,7 @@
 - [#1042](https://github.com/crypto-org-chain/cronos/pull/1042) call Close method on app to cleanup resource on graceful shutdown ([ethermint commit](https://github.com/crypto-org-chain/ethermint/commit/0ea7b86532a1144f229961f94b4524d5889e874d)).
 - [#1083](https://github.com/crypto-org-chain/cronos/pull/1083) memiavl support both sdk 46 and 47 root hash rules.
 - [#1091](https://github.com/crypto-org-chain/cronos/pull/1091) memiavl support rollback.
+- [#1100](https://github.com/crypto-org-chain/cronos/pull/1100) memiavl support read-only mode, and grab exclusive lock for write mode.
 
 ### Improvements
 

go.mod

@@ -190,6 +190,7 @@ require (
 	github.com/tklauser/numcpus v0.4.0 // indirect
 	github.com/tyler-smith/go-bip39 v1.1.0 // indirect
 	github.com/ulikunitz/xz v0.5.10 // indirect
+	github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d // indirect
 	github.com/zondax/hid v0.9.1 // indirect
 	github.com/zondax/ledger-go v0.14.1 // indirect
 	go.etcd.io/bbolt v1.3.7 // indirect

go.sum

@@ -1307,6 +1307,8 @@ github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
 github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
+github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d h1:XQyeLr7N9iY9mi+TGgsBFkj54+j3fdoo8e2u6zrGP5A=
+github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d/go.mod h1:hoMeDjlNXTNqVwrCk8YDyaBS2g5vFfEX2ezMi4vb6CY=
 github.com/zondax/hid v0.9.1 h1:gQe66rtmyZ8VeGFcOpbuH3r7erYtNEAezCAYu8LdkJo=
 github.com/zondax/hid v0.9.1/go.mod h1:l5wttcP0jwtdLjqjMMWFVEE7d1zO0jvSPA9OPZxWpEM=
 github.com/zondax/ledger-go v0.14.1 h1:Pip65OOl4iJ84WTpA4BKChvOufMhhbxED3BaihoZN4c=

gomod2nix.toml

@@ -538,6 +538,9 @@ schema = 3
   [mod."github.com/ulikunitz/xz"]
     version = "v0.5.10"
     hash = "sha256-bogOwQNmQVS7W+C7wci7XEUeYm9TB7PnxnyBIXKYbm0="
+  [mod."github.com/zbiljic/go-filelock"]
+    version = "v0.0.0-20170914061330-1dbf7103ab7d"
+    hash = "sha256-JqNj/Wg8nGFSmndgYC7+FZzL2zG7rwOQMjlqYs3ZGvw="
   [mod."github.com/zondax/hid"]
     version = "v0.9.1"
     hash = "sha256-hSVmN/f/lQHFhF60o6ej78ELC0MMoqQgqIX2hHjdTXg="

memiavl/db.go

@@ -14,7 +14,12 @@ import (
 	"github.com/tidwall/wal"
 )
 
-const DefaultSnapshotInterval = 1000
+const (
+	DefaultSnapshotInterval = 1000
+	LockFileName            = "LOCK"
+)
+
+var errReadOnly = errors.New("db is read-only")
 
 // DB implements DB-like functionalities on top of MultiTree:
 // - async snapshot rewriting
@@ -34,8 +39,10 @@ const DefaultSnapshotInterval = 1000
 // ```
 type DB struct {
 	MultiTree
-	dir    string
-	logger log.Logger
+	dir      string
+	logger   log.Logger
+	fileLock FileLock
+	readOnly bool
 
 	// result channel of snapshot rewrite goroutine
 	snapshotRewriteChan chan snapshotResult
@@ -72,6 +79,7 @@ type Options struct {
 	Logger          log.Logger
 	CreateIfMissing bool
 	InitialVersion  uint32
+	ReadOnly        bool
 	// the initial stores when initialize the empty instance
 	InitialStores      []string
 	SnapshotKeepRecent uint32
@@ -94,12 +102,56 @@ type Options struct {
 	LoadForOverwriting bool
 }
 
+func (opts Options) Validate() error {
+	if opts.ReadOnly && opts.CreateIfMissing {
+		return errors.New("can't create db in read-only mode")
+	}
+
+	if opts.ReadOnly && opts.LoadForOverwriting {
+		return errors.New("can't rollback db in read-only mode")
+	}
+
+	return nil
+}
+
+func (opts *Options) FillDefaults() {
+	if opts.Logger == nil {
+		opts.Logger = log.NewNopLogger()
+	}
+
+	if opts.SnapshotInterval == 0 {
+		opts.SnapshotInterval = DefaultSnapshotInterval
+	}
+}
+
 const (
 	SnapshotPrefix = "snapshot-"
 	SnapshotDirLen = len(SnapshotPrefix) + 20
 )
 
 func Load(dir string, opts Options) (*DB, error) {
+	if err := opts.Validate(); err != nil {
+		return nil, fmt.Errorf("invalid options: %w", err)
+	}
+	opts.FillDefaults()
+
+	if opts.CreateIfMissing {
+		if err := createDBIfNotExist(dir, opts.InitialVersion); err != nil {
+			return nil, fmt.Errorf("fail to load db: %w", err)
+		}
+	}
+
+	var (
+		err      error
+		fileLock FileLock
+	)
+	if !opts.ReadOnly {
+		fileLock, err = LockFile(filepath.Join(dir, LockFileName))
+		if err != nil {
+			return nil, fmt.Errorf("fail to lock db: %w", err)
+		}
+	}
+
 	snapshot := "current"
 	if opts.TargetVersion > 0 {
 		// find the biggest snapshot version that's less than or equal to the target version
@@ -113,15 +165,7 @@ func Load(dir string, opts Options) (*DB, error) {
 	path := filepath.Join(dir, snapshot)
 	mtree, err := LoadMultiTree(path, opts.ZeroCopy, opts.CacheSize)
 	if err != nil {
-		if opts.CreateIfMissing && os.IsNotExist(err) {
-			if err := initEmptyDB(dir, opts.InitialVersion); err != nil {
-				return nil, err
-			}
-			mtree, err = LoadMultiTree(path, opts.ZeroCopy, opts.CacheSize)
-		}
-		if err != nil {
-			return nil, err
-		}
+		return nil, err
 	}
 
 	wal, err := OpenWAL(walPath(dir), &wal.Options{NoCopy: true, NoSync: true})
@@ -176,6 +220,8 @@ func Load(dir string, opts Options) (*DB, error) {
 		MultiTree:                       *mtree,
 		logger:                          opts.Logger,
 		dir:                             dir,
+		fileLock:                        fileLock,
+		readOnly:                        opts.ReadOnly,
 		wal:                             wal,
 		walChanSize:                     opts.AsyncCommitBuffer,
 		snapshotKeepRecent:              opts.SnapshotKeepRecent,
@@ -184,15 +230,7 @@ func Load(dir string, opts Options) (*DB, error) {
 		triggerStateSyncExport:          opts.TriggerStateSyncExport,
 	}
 
-	if db.logger == nil {
-		db.logger = log.NewNopLogger()
-	}
-
-	if db.snapshotInterval == 0 {
-		db.snapshotInterval = DefaultSnapshotInterval
-	}
-
-	if db.Version() == 0 && len(opts.InitialStores) > 0 {
+	if !db.readOnly && db.Version() == 0 && len(opts.InitialStores) > 0 {
 		// do the initial upgrade with the `opts.InitialStores`
 		var upgrades []*TreeNameUpgrade
 		for _, name := range opts.InitialStores {
@@ -206,6 +244,11 @@ func Load(dir string, opts Options) (*DB, error) {
 	return db, nil
 }
 
+// ReadOnly returns whether the DB is opened in read-only mode.
+func (db *DB) ReadOnly() bool {
+	return db.readOnly
+}
+
 // SetInitialVersion wraps `MultiTree.SetInitialVersion`.
 // it do an immediate snapshot rewrite, because we can't use wal log to record this change,
 // because we need it to convert versions to wal index in the first place.
@@ -230,6 +273,10 @@ func (db *DB) ApplyUpgrades(upgrades []*TreeNameUpgrade) error {
 	db.mtx.Lock()
 	defer db.mtx.Unlock()
 
+	if db.readOnly {
+		return errReadOnly
+	}
+
 	if err := db.MultiTree.ApplyUpgrades(upgrades); err != nil {
 		return err
 	}
@@ -351,6 +398,10 @@ func (db *DB) Commit(changeSets []*NamedChangeSet) ([]byte, int64, error) {
 	db.mtx.Lock()
 	defer db.mtx.Unlock()
 
+	if db.readOnly {
+		return nil, 0, errReadOnly
+	}
+
 	if err := db.checkAsyncTasks(); err != nil {
 		return nil, 0, err
 	}
@@ -457,6 +508,10 @@ func (db *DB) RewriteSnapshot() error {
 	db.mtx.Lock()
 	defer db.mtx.Unlock()
 
+	if db.readOnly {
+		return errReadOnly
+	}
+
 	snapshotDir := snapshotName(db.lastCommitInfo.Version)
 	tmpDir := snapshotDir + "-tmp"
 	path := filepath.Join(db.dir, tmpDir)
@@ -525,6 +580,10 @@ func (db *DB) RewriteSnapshotBackground() error {
 	db.mtx.Lock()
 	defer db.mtx.Unlock()
 
+	if db.readOnly {
+		return errReadOnly
+	}
+
 	return db.rewriteSnapshotBackground()
 }
 
@@ -571,7 +630,17 @@ func (db *DB) Close() error {
 	db.mtx.Lock()
 	defer db.mtx.Unlock()
 
-	return errors.Join(db.waitAsyncCommit(), db.MultiTree.Close(), db.wal.Close())
+	errs := []error{
+		db.waitAsyncCommit(), db.MultiTree.Close(), db.wal.Close(),
+	}
+	db.wal = nil
+
+	if db.fileLock != nil {
+		errs = append(errs, db.fileLock.Unlock())
+		db.fileLock = nil
+	}
+
+	return errors.Join(errs...)
 }
 
 // TreeByName wraps MultiTree.TreeByName to add a lock.
@@ -611,6 +680,10 @@ func (db *DB) ApplyChangeSet(changeSets []*NamedChangeSet, updateCommitInfo bool
 	db.mtx.Lock()
 	defer db.mtx.Unlock()
 
+	if db.readOnly {
+		return nil, 0, errReadOnly
+	}
+
 	return db.MultiTree.ApplyChangeSet(changeSets, updateCommitInfo)
 }
 
@@ -793,6 +866,15 @@ func atomicRemoveDir(path string) error {
 	return os.RemoveAll(tmpPath)
 }
 
+// createDBIfNotExist detects if db does not exist and try to initialize an empty one.
+func createDBIfNotExist(dir string, initialVersion uint32) error {
+	_, err := os.Stat(filepath.Join(dir, "current", MetadataFileName))
+	if err != nil && os.IsNotExist(err) {
+		return initEmptyDB(dir, initialVersion)
+	}
+	return nil
+}
+
 type walEntry struct {
 	index uint64
 	data  *WALEntry

memiavl/db_test.go

@@ -72,8 +72,8 @@ func TestRewriteSnapshotBackground(t *testing.T) {
 	entries, err := os.ReadDir(db.dir)
 	require.NoError(t, err)
 
-	// three files: snapshot, current link, wal
-	require.Equal(t, 3, len(entries))
+	// three files: snapshot, current link, wal, LOCK
+	require.Equal(t, 4, len(entries))
 }
 
 func TestWAL(t *testing.T) {
@@ -230,6 +230,7 @@ func TestLoadVersion(t *testing.T) {
 		}
 		tmp, err := Load(dir, Options{
 			TargetVersion: uint32(v),
+			ReadOnly:      true,
 		})
 		require.NoError(t, err)
 		require.Equal(t, RefHashes[v-1], tmp.TreeByName("test").RootHash())
@@ -313,3 +314,41 @@ func TestEmptyValue(t *testing.T) {
 	require.Equal(t, version, db.Version())
 	require.Equal(t, hash, db.LastCommitInfo().CommitID().Hash)
 }
+
+func TestInvalidOptions(t *testing.T) {
+	dir := t.TempDir()
+
+	_, err := Load(dir, Options{ReadOnly: true})
+	require.Error(t, err)
+
+	_, err = Load(dir, Options{ReadOnly: true, CreateIfMissing: true})
+	require.Error(t, err)
+
+	db, err := Load(dir, Options{CreateIfMissing: true})
+	require.NoError(t, err)
+	require.NoError(t, db.Close())
+
+	_, err = Load(dir, Options{LoadForOverwriting: true, ReadOnly: true})
+	require.Error(t, err)
+
+	_, err = Load(dir, Options{ReadOnly: true})
+	require.NoError(t, err)
+}
+
+func TestExclusiveLock(t *testing.T) {
+	dir := t.TempDir()
+
+	db, err := Load(dir, Options{CreateIfMissing: true})
+	require.NoError(t, err)
+
+	_, err = Load(dir, Options{})
+	require.Error(t, err)
+
+	_, err = Load(dir, Options{ReadOnly: true})
+	require.NoError(t, err)
+
+	require.NoError(t, db.Close())
+
+	_, err = Load(dir, Options{})
+	require.NoError(t, err)
+}

memiavl/export.go

@@ -28,6 +28,7 @@ func (db *DB) Snapshot(height uint64, protoWriter protoio.Writer) (returnErr err
 		db, err := Load(db.dir, Options{
 			TargetVersion: version,
 			ZeroCopy:      true,
+			ReadOnly:      true,
 		})
 		if err != nil {
 			return errors.Wrapf(err, "invalid height: %d", height)

memiavl/filelock.go

@@ -0,0 +1,27 @@
+package memiavl
+
+import (
+	"path/filepath"
+
+	"github.com/zbiljic/go-filelock"
+)
+
+type FileLock interface {
+	Unlock() error
+}
+
+func LockFile(fname string) (FileLock, error) {
+	path, err := filepath.Abs(fname)
+	if err != nil {
+		return nil, err
+	}
+	fl, err := filelock.New(path)
+	if err != nil {
+		return nil, err
+	}
+	if _, err := fl.TryLock(); err != nil {
+		return nil, err
+	}
+
+	return fl, nil
+}

memiavl/go.mod

@@ -17,6 +17,7 @@ require (
 	github.com/tidwall/btree v1.5.0
 	github.com/tidwall/gjson v1.10.2
 	github.com/tidwall/wal v1.1.7
+	github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d
 	golang.org/x/exp v0.0.0-20230310171629-522b1b587ee0
 	golang.org/x/sync v0.1.0
 	golang.org/x/sys v0.7.0

memiavl/go.sum

@@ -384,6 +384,8 @@ github.com/yudai/golcs v0.0.0-20170316035057-ecda9a501e82/go.mod h1:lgjkn3NuSvDf
 github.com/yudai/pp v2.0.1+incompatible/go.mod h1:PuxR/8QJ7cyCkFp/aUDS+JY727OFEZkTdatxwunjIkc=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
+github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d h1:XQyeLr7N9iY9mi+TGgsBFkj54+j3fdoo8e2u6zrGP5A=
+github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d/go.mod h1:hoMeDjlNXTNqVwrCk8YDyaBS2g5vFfEX2ezMi4vb6CY=
 github.com/zondax/hid v0.9.1 h1:gQe66rtmyZ8VeGFcOpbuH3r7erYtNEAezCAYu8LdkJo=
 github.com/zondax/ledger-go v0.14.1 h1:Pip65OOl4iJ84WTpA4BKChvOufMhhbxED3BaihoZN4c=
 go.etcd.io/bbolt v1.3.6 h1:/ecaJf0sk1l4l6V4awd65v2C3ILy7MSj+s/x1ADCIMU=

store/go.mod

@@ -111,6 +111,7 @@ require (
 	github.com/tidwall/pretty v1.2.0 // indirect
 	github.com/tidwall/tinylru v1.1.0 // indirect
 	github.com/tidwall/wal v1.1.7 // indirect
+	github.com/zbiljic/go-filelock v0.0.0-20170914061330-1dbf7103ab7d // indirect
 	github.com/zondax/hid v0.9.1 // indirect
 	github.com/zondax/ledger-go v0.14.1 // indirect
 	go.etcd.io/bbolt v1.3.6 // indirect