possible deadlock with walletpassphrase

xuzhitong commented at 2:26 pm on December 18, 2018: none

When i used the encrypted wallet to test transfer parallelly,i frequently got the trouble: The rpc server is not responding.My test script is like below: #!/bin/sh while [ 1 ] do ./bitcoin-cli listaddressgroupings>null ./bitcoin-cli walletpassphrase “!@#$%^&*” 2 ./bitcoin sendtoaddress “mhAW6mzs5DeCbzBUHXBi4U446Fdnr1z8U7” 0.01 done

When i open the dead lock trace macro and check the debug log,i found the last print is as below: 2018-12-17 08:27:58 ThreadRPCServer method=walletpassphrase 2018-12-17 08:27:58 LOCKCONTENTION: cs_main 2018-12-17 08:27:58 Locker: wallet/rpcwallet.cpp:2642 2018-12-17 08:27:58 LOCKCONTENTION: pWallet->cs_wallet 2018-12-17 08:27:58 Locker: wallet/rpcwallet.cpp:2607

The line no 2642 is walletpassphrase and 2607 is LockWallet. I checked the code,i found there is no log printed for this line in function “walletpassphrase”: “LogPrint(BCLog::RPC, “queue run of timer %s in %i seconds (using %s)\n”, name, nSeconds, timerInterface->Name());” So i guess the reason may be: When “walletpassphrase” is running and lock the cs_wallet,the last relock timer triggered and call “LockWallet”.“LockWallet” want to lock the cs_wallet but it has been locked by “walletpassphrase”,so “LockWallet” is blocked.When “walletpassphrase” run to call “RPCRunLater”,the function will erase the timer from the std::map “deadlineTimers”,but the timer’s timeout call is blocked,so the dead lock take place. Is my guess right? How to reslove the issue? Any help will be appreciative！

practicalswift commented at 5:07 pm on December 18, 2018: contributor

@xuzhitong

Thanks a lot for reporting!

What is the output of bitcoind --version | head -1?

Are you able to reproduce this consistently?

If you start a fresh bitcoind instance and run your test script; will the issue trigger?

promag commented at 0:34 am on December 19, 2018: member

I checked the code,i found there is no log printed for this line @xuzhitong you have to give us more details ☝️

meshcollider added the label RPC/REST/ZMQ on Dec 19, 2018

xuzhitong commented at 1:18 am on December 19, 2018: none

I checked the code,i found there is no log printed for this line

@xuzhitong you have to give us more details ☝️

OK.Sorry for not clear it. The line is in function “RPCRunLater” which called by “walletpassphrase” . The code is below: void RPCRunLater(const std::string& name, std::function<void(void)> func, int64_t nSeconds) { if (!timerInterface) throw JSONRPCError(RPC_INTERNAL_ERROR, “No timer handler registered for RPC”); deadlineTimers.erase(name); LogPrint(BCLog::RPC, “queue run of timer %s in %i seconds (using %s)\n”, name, nSeconds, timerInterface->Name()); deadlineTimers.emplace(name, std::unique_ptr(timerInterface->NewTimer(func, nSeconds1000))); } static UniValue walletpassphrase(const JSONRPCRequest& request) { std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); CWallet const pwallet = wallet.get();

if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
    return NullUniValue;
}
......
LOCK2(cs_main, pwallet->cs_wallet);
if (!pwallet->IsCrypted()) {
    throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called.");
}

// Note that the walletpassphrase is stored in request.params[0] which is not mlock()ed
SecureString strWalletPass;
strWalletPass.reserve(100);
// TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string)
// Alternately, find a way to make request.params[0] mlock()'d to begin with.
strWalletPass = request.params[0].get_str().c_str();

// Get the timeout
int64_t nSleepTime = request.params[1].get_int64();
// Timeout cannot be negative, otherwise it will relock immediately
if (nSleepTime < 0) {
    throw JSONRPCError(RPC_INVALID_PARAMETER, "Timeout cannot be negative.");
}
// Clamp timeout
constexpr int64_t MAX_SLEEP_TIME = 100000000; // larger values trigger a macos/libevent bug?
if (nSleepTime > MAX_SLEEP_TIME) {
    nSleepTime = MAX_SLEEP_TIME;
}

if (strWalletPass.length() > 0)
{
    if (!pwallet->Unlock(strWalletPass)) {
        throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect.");
    }
}
else
    throw std::runtime_error(
        "walletpassphrase <passphrase> <timeout>\n"
        "Stores the wallet decryption key in memory for <timeout> seconds.");

pwallet->TopUpKeyPool();

pwallet->nRelockTime = GetTime() + nSleepTime;
**RPCRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), std::bind(LockWallet, pwallet), nSleepTime);**

return NullUniValue;

}

xuzhitong commented at 1:25 am on December 19, 2018: none

@xuzhitong

Thanks a lot for reporting!

What is the output of bitcoind --version | head -1?

Are you able to reproduce this consistently?

If you start a fresh bitcoind instance and run your test script; will the issue trigger?

Yes.The issue is not difficulty to reproduce in my test condition.I will try the newest version later. I compared the newest version code,but I didn’t find any difference associated with “walletpassphrase”.So i think the issue may be also exist in the fresh “bitcoind”.

promag commented at 7:44 am on December 19, 2018: member

I mean, let us know what version you have.

xuzhitong commented at 11:56 am on December 19, 2018: none

I mean, let us know what version you have.

The version is 0.15.2.It’s must be too old.But for some reason,i couldn’t update the version to the newest.So if the issue has been repair in some version,please tell me,thanks very much!

practicalswift commented at 12:10 pm on December 19, 2018: contributor

@xuzhitong The probability that this has been fixed since 0.15.2 is quite high, so please try to reproduce against the current version. Thanks for reporting!

xuzhitong commented at 10:46 am on December 20, 2018: none

@xuzhitong The probability that this has been fixed since 0.15.2 is quite high, so please try to reproduce against the current version. Thanks for reporting!

I compared the two version code and tried to merge the code associated with httprpc mutithread part.I noticed the significant modify is changing the boost thread call to std thread call.The merged files list are below: sync.h,sync.cpp,httprpc.cpp,init.cpp,rpc/mining.cpp. Then I make and upgrade to test the same scenario,but unfortunately the issue is also exist. I think the issue caused by erasing a pending(waiting cs_wallet lock) timer from “deadlinetimes” map ,so i tried a fix using “evtimer_pending” and “EV_PERSIST” to check if the timer pending or active.If the timer is pending and the tv_out time is earlier than the current time,keep it in map,else,delete it.I found the issue has gone.I think it’s not a very good way.Please help to confirm the issue.Thanks very much!

promag commented at 11:34 pm on March 31, 2020: member

@xuzhitong I’m afraid 0.19.1 still has this problem. FYI #18487 might fix the issue. Thanks for reporting!

laanwj closed this on Apr 6, 2020

sidhujag referenced this in commit 82255a05ec on Apr 8, 2020

DrahtBot locked this on Feb 15, 2022

possible deadlock with walletpassphrase #14995