salvagewallet fails verification #7463
Comments
|
Another one at https://travis-ci.org/bitcoin/bitcoin/jobs/109329790 |
Check for EOF before every getline, and warn when reading gets to EOF before the end of the data. Stricter error checking could shed more light on issues such as bitcoin#7463 and bitcoin#7379.
|
I've noticed that this mostly happens on the 32 bit travis build (.3) |
|
I don't think we're even sure whether -salvagewallet terminates. In this case every time the last line seem to call some bitcoin-cli command with -rpcwait. In case the client would crash, or exit, it would just wait forever. |
The client does not crash nor exit, iIrc. salvagewallet returns some sort of error and the client tries to shutdown. Somehow this fails and the "write-peers.dat-thread" just continues to run. Haven't had the time to look into that yet... |
|
|
|
You can just run $ for i in {0..10000}; do qa/pull-tester/rpc-tests.py wallet --nocleanup;if [ $? -ne 0 ];then echo; echo "fail"; sleep 1d;fi ;doneand come back 20 minutes later to dig through the dumps. |
|
I did a similar thing, I put the part of wallet.py that does the salvage in an infinite loop. Managed to make it hang after ~100 iterations. Looking at the debug.log it didn't hang during the salvage, but manages to go into normal operation. And the node didn't start shutting down at all. I could even succesfully send it a stop command and it stopped running. This won't set the python script in motion again though. Curious.
Oh, I didn't notice it spawns three nodes. The third one indeed has: |
|
Ok, I was right in my gist above. I can reproduce this 100% of the time now with a specific wallet: The test framework somehow doesn't detect this sudden exit. I think it will try to send a stop command (with -rpcwait) which never arrives, because the node is not running anymore in the first place. It will try forever to send it. This makes the test hang 100% of the time: --- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -390,8 +390,9 @@ bool CWallet::Verify(const string& walletFile, string& warningString, string& er
if (GetBoolArg("-salvagewallet", false))
{
// Recover readable keypairs:
- if (!CWalletDB::Recover(bitdb, walletFile, true))
- return false;
+ return false;
}
if (boost::filesystem::exists(GetDataDir() / walletFile))That, of course, leaves the question why the wallet ends up in a non-salvageable state. But at least the test should not hang in this case, but fail with a clearer error. |
|
#7744 solves the test framework issue.
The underlying problem is weird. It seems that randomly, after exit, the wallet database fails verification with returns
Here's an example wallet that triggers this: https://dev.visucore.com/bitcoin/tmp/crapwallet.dat |
Replace the `bitcoin-cli -rpcwait` after spawning bitcoind with our own loop that detects when bitcoind exits prematurely. And if one node fails to start, stop the others. This prevents a hang in such a case (see bitcoin#7463).
maflcko
changed the title
|
This test will be disabled in #8038 until this is be fixed. |
This is a merge of bitcoin/bitcoin#7744 / MVF-Core PR#16. Instead of freezing up qa tests, wallet.py will fail on some 32-bit platforms where wallet salvage operation sometimes fails due to unknown reasons. See also: bitcoin/bitcoin#7463 about the -salvagewallet problem. Core has decided to disable the salvage part of the test in bitcoin/bitcoin#8038 , we may implement a Skip() of this test on 32-bit or disable that test step in a similar way. The wallet salvage problem is definitely unrelated to any fork programming.
* Disable salvagewallet in GUI salvagewallet is known to cause problems and it's way to powerful to let it be accessible in GUI bitcoin#7463 I have few reports from different users as well, would be a disaster for them, glad we have autobackup... * typo
Check for EOF before every getline, and warn when reading gets to EOF before the end of the data. Stricter error checking could shed more light on issues such as bitcoin#7463 and bitcoin#7379.
|
I've been digging into this failure over the last couple of days. This is what I've found so far:
(from the BDB 4.8.30 docs)
the output format for BDB should be key in one row followed by value in the next row. You can see here that the final
(you can see at the end that it's reading keys as values and vice versa). After running with Next steps:
|
|
Proposed fix here: #10540 |
* Disable salvagewallet in GUI salvagewallet is known to cause problems and it's way to powerful to let it be accessible in GUI bitcoin/bitcoin#7463 I have few reports from different users as well, would be a disaster for them, glad we have autobackup... * typo
Check for EOF before every getline, and warn when reading gets to EOF before the end of the data. Stricter error checking could shed more light on issues such as bitcoin#7463 and bitcoin#7379.
|
Just linking to related comments for reference: #10540 (comment) and #10991 |
|
I had somehow missed @jnewbery's investigation here. Thanks for looking at this closely and figuring out the underlying issue. |
Check for EOF before every getline, and warn when reading gets to EOF before the end of the data. Stricter error checking could shed more light on issues such as bitcoin#7463 and bitcoin#7379.
Check for EOF before every getline, and warn when reading gets to EOF before the end of the data. Stricter error checking could shed more light on issues such as bitcoin#7463 and bitcoin#7379.
Check for EOF before every getline, and warn when reading gets to EOF before the end of the data. Stricter error checking could shed more light on issues such as bitcoin#7463 and bitcoin#7379.
Check for EOF before every getline, and warn when reading gets to EOF before the end of the data. Stricter error checking could shed more light on issues such as bitcoin#7463 and bitcoin#7379.
|
|
and others
This is somewhat rare but still worth to take a look sometime. I can reproduce locally and travis complains in
wallet.py:Edit:
This will now show up as
The text was updated successfully, but these errors were encountered: