From b3cad7b79d20d281e4a8b79d70a5147f294d0461 Mon Sep 17 00:00:00 2001 From: asamuj Date: Tue, 6 Aug 2024 10:42:43 +0800 Subject: [PATCH 1/2] fix dsbackend lock --- pkg/wallet/dsbackend.go | 36 ++++++++++++++++++-------------- tools/conformance/chaos/actor.go | 6 +++--- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/pkg/wallet/dsbackend.go b/pkg/wallet/dsbackend.go index 2ff8ca6e8e..0e5881d721 100644 --- a/pkg/wallet/dsbackend.go +++ b/pkg/wallet/dsbackend.go @@ -45,7 +45,7 @@ type DSBackend struct { cache map[address.Address]struct{} - PassphraseConf config.PassphraseConfig + passphraseConf config.PassphraseConfig password *memguard.Enclave unLocked map[address.Address]*key.KeyInfo @@ -81,7 +81,7 @@ func NewDSBackend(ctx context.Context, ds repo.Datastore, passphraseCfg config.P backend := &DSBackend{ ds: ds, cache: addrCache, - PassphraseConf: passphraseCfg, + passphraseConf: passphraseCfg, unLocked: make(map[address.Address]*key.KeyInfo, len(addrCache)), } @@ -104,11 +104,11 @@ func (backend *DSBackend) Addresses(ctx context.Context) []address.Address { backend.lk.RLock() defer backend.lk.RUnlock() - var cpy []address.Address + var addrs []address.Address for addr := range backend.cache { - cpy = append(cpy, addr) + addrs = append(addrs, addr) } - return cpy + return addrs } // HasAddress checks if the passed in address is stored in this backend. @@ -188,20 +188,21 @@ func (backend *DSBackend) putKeyInfo(ctx context.Context, ki *key.KeyInfo) error } var keyJSON []byte - err = backend.UsePassword(func(password []byte) error { - var err error - keyJSON, err = encryptKey(key, password, backend.PassphraseConf.ScryptN, backend.PassphraseConf.ScryptP) + if err := backend.UsePassword(func(password []byte) error { + keyJSON, err = encryptKey(key, password, backend.passphraseConf.ScryptN, backend.passphraseConf.ScryptP) return err - }) - if err != nil { + }); err != nil { return err } if err := backend.ds.Put(ctx, ds.NewKey(key.Address.String()), keyJSON); err != nil { return errors.Wrapf(err, "failed to store new address: %s", key.Address.String()) } + + backend.lk.Lock() backend.cache[addr] = struct{}{} backend.unLocked[addr] = ki + backend.lk.Unlock() return nil } @@ -284,6 +285,8 @@ func (backend *DSBackend) getKey(ctx context.Context, addr address.Address, pass } func (backend *DSBackend) LockWallet(ctx context.Context) error { + backend.lk.Lock() + defer backend.lk.Unlock() if backend.state == Lock { return fmt.Errorf("already locked") } @@ -293,9 +296,7 @@ func (backend *DSBackend) LockWallet(ctx context.Context) error { } for _, addr := range backend.Addresses(ctx) { - backend.lk.Lock() delete(backend.unLocked, addr) - backend.lk.Unlock() } backend.cleanPassword() backend.state = Lock @@ -305,7 +306,9 @@ func (backend *DSBackend) LockWallet(ctx context.Context) error { // UnLockWallet unlock wallet with password, decrypt local key in db and save to protected memory func (backend *DSBackend) UnLockWallet(ctx context.Context, password []byte) error { + backend.lk.Lock() defer func() { + backend.lk.Unlock() for i := range password { password[i] = 0 } @@ -324,9 +327,7 @@ func (backend *DSBackend) UnLockWallet(ctx context.Context, password []byte) err return err } - backend.lk.Lock() backend.unLocked[addr] = ki - backend.lk.Unlock() } backend.state = Unlock @@ -335,6 +336,8 @@ func (backend *DSBackend) UnLockWallet(ctx context.Context, password []byte) err // SetPassword set password for wallet , and wallet used this password to encrypt private key func (backend *DSBackend) SetPassword(ctx context.Context, password []byte) error { + backend.lk.Lock() + defer backend.lk.Unlock() if backend.password != nil { return ErrRepeatPassword } @@ -344,9 +347,8 @@ func (backend *DSBackend) SetPassword(ctx context.Context, password []byte) erro if err != nil { return err } - backend.lk.Lock() + backend.unLocked[addr] = ki - backend.lk.Unlock() } if backend.state == undetermined { backend.state = Unlock @@ -364,6 +366,8 @@ func (backend *DSBackend) HasPassword() bool { // WalletState return wallet state(lock/unlock) func (backend *DSBackend) WalletState(ctx context.Context) int { + backend.lk.Lock() + defer backend.lk.Unlock() return backend.state } diff --git a/tools/conformance/chaos/actor.go b/tools/conformance/chaos/actor.go index d054246384..ce5b78e90c 100644 --- a/tools/conformance/chaos/actor.go +++ b/tools/conformance/chaos/actor.go @@ -15,12 +15,12 @@ import ( //go:generate go run ./gen -// Actor is a chaos actor. It implements a variety of illegal behaviours that -// trigger violations of VM invariants. These behaviours are not found in +// Actor is a chaos actor. It implements a variety of illegal behaviors that +// trigger violations of VM invariants. These behaviors are not found in // production code, but are important to test that the VM constraints are // properly enforced. // -// The chaos actor is being incubated and its behaviour and ABI be standardised +// The chaos actor is being incubated and its behavior and ABI be standardized // shortly. Its CID is ChaosActorCodeCID, and its singleton address is 98 (Address). // It cannot be instantiated via the init actor, and its constructor panics. // From bf4562db71842181cfbcddc87c9de0cf7fc9b8c0 Mon Sep 17 00:00:00 2001 From: asamuj Date: Tue, 6 Aug 2024 14:01:51 +0800 Subject: [PATCH 2/2] Replace mutex with atomic value --- pkg/wallet/dsbackend.go | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/pkg/wallet/dsbackend.go b/pkg/wallet/dsbackend.go index 0e5881d721..7f6b5a6bf1 100644 --- a/pkg/wallet/dsbackend.go +++ b/pkg/wallet/dsbackend.go @@ -7,6 +7,7 @@ import ( "reflect" "strings" "sync" + "sync/atomic" "github.com/awnumar/memguard" "github.com/filecoin-project/go-address" @@ -50,7 +51,7 @@ type DSBackend struct { password *memguard.Enclave unLocked map[address.Address]*key.KeyInfo - state int + state atomic.Int64 } var _ Backend = (*DSBackend)(nil) @@ -198,11 +199,8 @@ func (backend *DSBackend) putKeyInfo(ctx context.Context, ki *key.KeyInfo) error if err := backend.ds.Put(ctx, ds.NewKey(key.Address.String()), keyJSON); err != nil { return errors.Wrapf(err, "failed to store new address: %s", key.Address.String()) } - - backend.lk.Lock() backend.cache[addr] = struct{}{} backend.unLocked[addr] = ki - backend.lk.Unlock() return nil } @@ -285,9 +283,7 @@ func (backend *DSBackend) getKey(ctx context.Context, addr address.Address, pass } func (backend *DSBackend) LockWallet(ctx context.Context) error { - backend.lk.Lock() - defer backend.lk.Unlock() - if backend.state == Lock { + if backend.state.Load() == Lock { return fmt.Errorf("already locked") } @@ -296,24 +292,24 @@ func (backend *DSBackend) LockWallet(ctx context.Context) error { } for _, addr := range backend.Addresses(ctx) { + backend.lk.Lock() delete(backend.unLocked, addr) + backend.lk.Unlock() } backend.cleanPassword() - backend.state = Lock + backend.state.Store(Lock) return nil } // UnLockWallet unlock wallet with password, decrypt local key in db and save to protected memory func (backend *DSBackend) UnLockWallet(ctx context.Context, password []byte) error { - backend.lk.Lock() defer func() { - backend.lk.Unlock() for i := range password { password[i] = 0 } }() - if backend.state == Unlock { + if backend.state.Load() == Unlock { return fmt.Errorf("already unlocked") } @@ -327,17 +323,17 @@ func (backend *DSBackend) UnLockWallet(ctx context.Context, password []byte) err return err } + backend.lk.Lock() backend.unLocked[addr] = ki + backend.lk.Unlock() } - backend.state = Unlock + backend.state.Store(Unlock) return nil } // SetPassword set password for wallet , and wallet used this password to encrypt private key func (backend *DSBackend) SetPassword(ctx context.Context, password []byte) error { - backend.lk.Lock() - defer backend.lk.Unlock() if backend.password != nil { return ErrRepeatPassword } @@ -347,13 +343,12 @@ func (backend *DSBackend) SetPassword(ctx context.Context, password []byte) erro if err != nil { return err } - + backend.lk.Lock() backend.unLocked[addr] = ki - } - if backend.state == undetermined { - backend.state = Unlock + backend.lk.Unlock() } + backend.state.CompareAndSwap(undetermined, Unlock) backend.setPassword(password) return nil @@ -366,9 +361,7 @@ func (backend *DSBackend) HasPassword() bool { // WalletState return wallet state(lock/unlock) func (backend *DSBackend) WalletState(ctx context.Context) int { - backend.lk.Lock() - defer backend.lk.Unlock() - return backend.state + return int(backend.state.Load()) } func (backend *DSBackend) setPassword(password []byte) {