Browse Source

Fix deadlocks in setaccount, sendfrom RPC calls

SendMoney*() now requires caller to acquire cs_main.
GetAccountAddress() now requires caller to acquire cs_main, cs_mapWallet.

Ordering is intended to match these two callchains[1]:

1. CRITICAL_BLOCK(cs_main)
    ProcessMessage(pfrom, strCommand, vMsg)
        AddToWalletIfMine()
              AddToWallet(wtx)
                  CRITICAL_BLOCK(cs_mapWallet)

2. CRITICAL_BLOCK(cs_main)
    ProcessMessage(pfrom, strCommand, vMsg)
        AddToWalletIfMine()
              AddToWallet(wtx)
                  CRITICAL_BLOCK(cs_mapWallet)
                      walletdb.WriteName(PubKeyToAddress(vchDefaultKey), "")
                          CRITICAL_BLOCK(cs_mapAddressBook)

Spotted by ArtForz.  Additional deadlock fixes by Gavin.

[1] http://www.bitcoin.org/smf/index.php?topic=4904.msg71897#msg71897
miguelfreitas
Jeff Garzik 14 years ago committed by Jeff Garzik
parent
commit
f5f1878ba1
  1. 2
      db.cpp
  2. 6
      main.cpp
  3. 22
      rpc.cpp
  4. 3
      ui.cpp

2
db.cpp

@ -668,8 +668,8 @@ bool CWalletDB::LoadWallet() @@ -668,8 +668,8 @@ bool CWalletDB::LoadWallet()
#endif
//// todo: shouldn't we catch exceptions and try to recover and continue?
CRITICAL_BLOCK(cs_mapKeys)
CRITICAL_BLOCK(cs_mapWallet)
CRITICAL_BLOCK(cs_mapKeys)
{
// Get cursor
Dbc* pcursor = GetCursor();

6
main.cpp

@ -4046,9 +4046,8 @@ bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey) @@ -4046,9 +4046,8 @@ bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey)
// requires cs_main lock
string SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, bool fAskFee)
{
CRITICAL_BLOCK(cs_main)
{
CReserveKey reservekey;
int64 nFeeRequired;
@ -4068,13 +4067,14 @@ string SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, bool fAs @@ -4068,13 +4067,14 @@ string SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, bool fAs
if (!CommitTransaction(wtxNew, reservekey))
return _("Error: The transaction was rejected. This might happen if some of the coins in your wallet were already spent, such as if you used a copy of wallet.dat and coins were spent in the copy but not marked as spent here.");
}
MainFrameRepaint();
return "";
}
// requires cs_main lock
string SendMoneyToBitcoinAddress(string strAddress, int64 nValue, CWalletTx& wtxNew, bool fAskFee)
{
// Check amount

22
rpc.cpp

@ -315,12 +315,11 @@ Value getnewaddress(const Array& params, bool fHelp) @@ -315,12 +315,11 @@ Value getnewaddress(const Array& params, bool fHelp)
}
// requires cs_main, cs_mapWallet locks
string GetAccountAddress(string strAccount, bool bForceNew=false)
{
string strAddress;
CRITICAL_BLOCK(cs_mapWallet)
{
CWalletDB walletdb;
walletdb.TxnBegin();
@ -354,7 +353,7 @@ string GetAccountAddress(string strAccount, bool bForceNew=false) @@ -354,7 +353,7 @@ string GetAccountAddress(string strAccount, bool bForceNew=false)
walletdb.TxnCommit();
strAddress = PubKeyToAddress(account.vchPubKey);
}
return strAddress;
}
@ -368,7 +367,15 @@ Value getaccountaddress(const Array& params, bool fHelp) @@ -368,7 +367,15 @@ Value getaccountaddress(const Array& params, bool fHelp)
// Parse the account first so we don't generate a key if there's an error
string strAccount = AccountFromValue(params[0]);
return GetAccountAddress(strAccount);
Value ret;
CRITICAL_BLOCK(cs_main)
CRITICAL_BLOCK(cs_mapWallet)
{
ret = GetAccountAddress(strAccount);
}
return ret;
}
@ -392,6 +399,8 @@ Value setaccount(const Array& params, bool fHelp) @@ -392,6 +399,8 @@ Value setaccount(const Array& params, bool fHelp)
strAccount = AccountFromValue(params[1]);
// Detect when changing the account of an address that is the 'unused current key' of another account:
CRITICAL_BLOCK(cs_main)
CRITICAL_BLOCK(cs_mapWallet)
CRITICAL_BLOCK(cs_mapAddressBook)
{
if (mapAddressBook.count(strAddress))
@ -475,9 +484,13 @@ Value sendtoaddress(const Array& params, bool fHelp) @@ -475,9 +484,13 @@ Value sendtoaddress(const Array& params, bool fHelp)
if (params.size() > 3 && params[3].type() != null_type && !params[3].get_str().empty())
wtx.mapValue["to"] = params[3].get_str();
CRITICAL_BLOCK(cs_main)
{
string strError = SendMoneyToBitcoinAddress(strAddress, nAmount, wtx);
if (strError != "")
throw JSONRPCError(-4, strError);
}
return wtx.GetHash().GetHex();
}
@ -752,6 +765,7 @@ Value sendfrom(const Array& params, bool fHelp) @@ -752,6 +765,7 @@ Value sendfrom(const Array& params, bool fHelp)
if (params.size() > 5 && params[5].type() != null_type && !params[5].get_str().empty())
wtx.mapValue["to"] = params[5].get_str();
CRITICAL_BLOCK(cs_main)
CRITICAL_BLOCK(cs_mapWallet)
{
// Check funds

3
ui.cpp

@ -1933,6 +1933,8 @@ void CSendDialog::OnButtonSend(wxCommandEvent& event) @@ -1933,6 +1933,8 @@ void CSendDialog::OnButtonSend(wxCommandEvent& event)
bool fBitcoinAddress = AddressToHash160(strAddress, hash160);
if (fBitcoinAddress)
{
CRITICAL_BLOCK(cs_main)
{
// Send to bitcoin address
CScript scriptPubKey;
@ -1949,6 +1951,7 @@ void CSendDialog::OnButtonSend(wxCommandEvent& event) @@ -1949,6 +1951,7 @@ void CSendDialog::OnButtonSend(wxCommandEvent& event)
EndModal(false);
}
}
}
else
{
// Parse IP address

Loading…
Cancel
Save