Skip to content

Commit

Permalink
dcr: Avoid freeze inside db transaction. (crypto-power#493)
Browse files Browse the repository at this point in the history
In dcr there's a request for the current wallet inside a db
transaction. If a user closes a wallet during
*asset.IndexTransactions() It will get the lock at
*dcrLoader.UnloadWallet() but then get stuck at l.db.Close() because it
cannot close the db while a transation is taking place. So the request
in the transation gets stuck trying to get the lock. Prevent that by
holding a db lock while indexing transactions that stops the wallet
from being unloaded during that time.
  • Loading branch information
JoeGruffins authored May 6, 2024
1 parent d56b981 commit d34f1f2
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 5 deletions.
3 changes: 3 additions & 0 deletions libwallet/assets/dcr/txindex.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ func (asset *Asset) IndexTransactions() error {
return utils.ErrDCRNotInitialized
}

asset.dbMutex.Lock()
defer asset.dbMutex.Unlock()

ctx, _ := asset.ShutdownContextWithCancel()

var totalIndex int32
Expand Down
23 changes: 18 additions & 5 deletions libwallet/assets/dcr/wallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,17 @@ type Asset struct {
accountMixerNotificationListeners map[string]*AccountMixerNotificationListener
txAndBlockNotificationListeners map[string]*sharedW.TxAndBlockNotificationListener
blocksRescanProgressListener *sharedW.BlocksRescanProgressListener

// dbMutex should be held when db transactions would circle back around
// and hold the mu lock to prevent a freeze.
dbMutex *sync.Mutex
}

// Verify that DCR implements the shared assets interface.
var _ sharedW.Asset = (*Asset)(nil)

// initWalletLoader setups the loader.
func initWalletLoader(chainParams *chaincfg.Params, rootdir, walletDbDriver string) loader.AssetLoader {
func initWalletLoader(chainParams *chaincfg.Params, rootdir, walletDbDriver string, dbMutex *sync.Mutex) loader.AssetLoader {
// TODO: Allow users provide values to override these defaults.
cfg := &sharedW.WConfig{
GapLimit: 20,
Expand Down Expand Up @@ -82,6 +86,7 @@ func initWalletLoader(chainParams *chaincfg.Params, rootdir, walletDbDriver stri
ManualTickets: cfg.ManualTickets,
AccountGapLimit: cfg.AccountGapLimit,
MixSplitLimit: cfg.MixSplitLimit,
DBMutex: dbMutex,
}
walletLoader := dcr.NewLoader(loaderCfg)

Expand All @@ -105,7 +110,8 @@ func CreateNewWallet(pass *sharedW.AuthInfo, params *sharedW.InitParams) (shared
return nil, err
}

ldr := initWalletLoader(chainParams, params.RootDir, params.DbDriver)
var dbMutex sync.Mutex
ldr := initWalletLoader(chainParams, params.RootDir, params.DbDriver, &dbMutex)

w, err := sharedW.CreateNewWallet(pass, ldr, params, utils.DCRWalletAsset)
if err != nil {
Expand All @@ -121,6 +127,7 @@ func CreateNewWallet(pass *sharedW.AuthInfo, params *sharedW.InitParams) (shared
txAndBlockNotificationListeners: make(map[string]*sharedW.TxAndBlockNotificationListener),
accountMixerNotificationListeners: make(map[string]*AccountMixerNotificationListener),
vspClients: make(map[string]*vsp.Client),
dbMutex: &dbMutex,
}

dcrWallet.SetNetworkCancelCallback(dcrWallet.SafelyCancelSync)
Expand All @@ -142,7 +149,8 @@ func CreateWatchOnlyWallet(walletName, extendedPublicKey string, params *sharedW
return nil, err
}

ldr := initWalletLoader(chainParams, params.RootDir, params.DbDriver)
var dbMutex sync.Mutex
ldr := initWalletLoader(chainParams, params.RootDir, params.DbDriver, &dbMutex)
w, err := sharedW.CreateWatchOnlyWallet(walletName, extendedPublicKey,
ldr, params, utils.DCRWalletAsset)
if err != nil {
Expand All @@ -157,6 +165,7 @@ func CreateWatchOnlyWallet(walletName, extendedPublicKey string, params *sharedW
},
txAndBlockNotificationListeners: make(map[string]*sharedW.TxAndBlockNotificationListener),
accountMixerNotificationListeners: make(map[string]*AccountMixerNotificationListener),
dbMutex: &dbMutex,
}

dcrWallet.SetNetworkCancelCallback(dcrWallet.SafelyCancelSync)
Expand All @@ -177,7 +186,8 @@ func RestoreWallet(seedMnemonic string, pass *sharedW.AuthInfo, params *sharedW.
return nil, err
}

ldr := initWalletLoader(chainParams, params.RootDir, params.DbDriver)
var dbMutex sync.Mutex
ldr := initWalletLoader(chainParams, params.RootDir, params.DbDriver, &dbMutex)
w, err := sharedW.RestoreWallet(seedMnemonic, pass, ldr, params, utils.DCRWalletAsset)
if err != nil {
return nil, err
Expand All @@ -192,6 +202,7 @@ func RestoreWallet(seedMnemonic string, pass *sharedW.AuthInfo, params *sharedW.
vspClients: make(map[string]*vsp.Client),
txAndBlockNotificationListeners: make(map[string]*sharedW.TxAndBlockNotificationListener),
accountMixerNotificationListeners: make(map[string]*AccountMixerNotificationListener),
dbMutex: &dbMutex,
}

dcrWallet.SetNetworkCancelCallback(dcrWallet.SafelyCancelSync)
Expand All @@ -212,7 +223,8 @@ func LoadExisting(w *sharedW.Wallet, params *sharedW.InitParams) (sharedW.Asset,
return nil, err
}

ldr := initWalletLoader(chainParams, params.RootDir, params.DbDriver)
var dbMutex sync.Mutex
ldr := initWalletLoader(chainParams, params.RootDir, params.DbDriver, &dbMutex)
dcrWallet := &Asset{
Wallet: w,
vspClients: make(map[string]*vsp.Client),
Expand All @@ -222,6 +234,7 @@ func LoadExisting(w *sharedW.Wallet, params *sharedW.InitParams) (sharedW.Asset,
},
txAndBlockNotificationListeners: make(map[string]*sharedW.TxAndBlockNotificationListener),
accountMixerNotificationListeners: make(map[string]*AccountMixerNotificationListener),
dbMutex: &dbMutex,
}

err = dcrWallet.Prepare(ldr, params)
Expand Down
11 changes: 11 additions & 0 deletions libwallet/internal/loader/dcr/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ type dcrLoader struct {
mixSplitLimit int

mu sync.RWMutex

// dbMutex should be held when db transactions would circle back around
// and hold the mu lock to prevent a freeze.
dbMutex *sync.Mutex
}

// StakeOptions contains the various options necessary for stake mining.
Expand Down Expand Up @@ -77,6 +81,7 @@ type LoaderConf struct {
ManualTickets bool
AccountGapLimit int
MixSplitLimit int
DBMutex *sync.Mutex
}

// NewLoader constructs a DCR Loader.
Expand All @@ -91,6 +96,7 @@ func NewLoader(cfg *LoaderConf) loader.AssetLoader {
manualTickets: cfg.ManualTickets,
relayFee: cfg.RelayFee,
mixSplitLimit: cfg.MixSplitLimit,
dbMutex: cfg.DBMutex,

Loader: loader.NewLoader(cfg.DBDirPath),
}
Expand Down Expand Up @@ -342,6 +348,11 @@ func (l *dcrLoader) GetLoadedWallet() (*loader.LoadedWallets, bool) {
func (l *dcrLoader) UnloadWallet() error {
const op errors.Op = "loader.UnloadWallet"

// Hold the db mutex before holding the loader mutex. This allows
// callers to call *dcrLoader methods inside of db transactions.
l.dbMutex.Lock()
defer l.dbMutex.Unlock()

defer l.mu.Unlock()
l.mu.Lock()

Expand Down

0 comments on commit d34f1f2

Please sign in to comment.