From d34f1f2f41b9decf1063f52e885d91ea7d520c4b Mon Sep 17 00:00:00 2001 From: JoeGruffins <34998433+JoeGruffins@users.noreply.github.com> Date: Mon, 6 May 2024 20:11:28 +0900 Subject: [PATCH] dcr: Avoid freeze inside db transaction. (#493) 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. --- libwallet/assets/dcr/txindex.go | 3 +++ libwallet/assets/dcr/wallet.go | 23 ++++++++++++++++++----- libwallet/internal/loader/dcr/loader.go | 11 +++++++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/libwallet/assets/dcr/txindex.go b/libwallet/assets/dcr/txindex.go index ad8e98bfc..b15eaee26 100644 --- a/libwallet/assets/dcr/txindex.go +++ b/libwallet/assets/dcr/txindex.go @@ -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 diff --git a/libwallet/assets/dcr/wallet.go b/libwallet/assets/dcr/wallet.go index 1615a6d89..a479de8f7 100644 --- a/libwallet/assets/dcr/wallet.go +++ b/libwallet/assets/dcr/wallet.go @@ -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, @@ -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) @@ -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 { @@ -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) @@ -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 { @@ -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) @@ -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 @@ -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) @@ -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), @@ -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) diff --git a/libwallet/internal/loader/dcr/loader.go b/libwallet/internal/loader/dcr/loader.go index ead8ac3f1..1d3828c65 100644 --- a/libwallet/internal/loader/dcr/loader.go +++ b/libwallet/internal/loader/dcr/loader.go @@ -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. @@ -77,6 +81,7 @@ type LoaderConf struct { ManualTickets bool AccountGapLimit int MixSplitLimit int + DBMutex *sync.Mutex } // NewLoader constructs a DCR Loader. @@ -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), } @@ -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()