Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Race leading to LMDB assert failed on wallet creation #3030

Closed
PlasmaPower opened this issue Nov 1, 2020 · 2 comments
Closed

Race leading to LMDB assert failed on wallet creation #3030

PlasmaPower opened this issue Nov 1, 2020 · 2 comments
Assignees

Comments

@PlasmaPower
Copy link
Contributor

Description of bug:

Occasional crash when creating a wallet, due to yet another bad side effect of the dual wallet memory and LMDB storage.

Steps to reproduce the issue:

Can't be reliably reproduced, but try creating wallets en masse during bootstrapping or high tps.

Describe the results you received:

Assertion (status == 0) failed
nano::mdb_iterator<T, U>::mdb_iterator(const nano::transaction&, MDB_dbi, const MDB_val&) [with T = nano::public_key; U = nano::wallet_value; MDB_dbi = unsigned int; MDB_val = MDB_val]
/home/lee/programming/cpp/nano-node/nano/node/lmdb/lmdb_iterator.hpp:39

Describe the results you expected:

The node to not crash

Additional information you deem important (e.g. issue happens only occasionally):

I believe I've traced this issue. The generic problem is the following situation:

  • Wallet transaction A created
  • Wallet transaction B created
  • Wallet created with transaction B
  • Transaction B committed
  • Wallet saved to memory
  • Attempt to access wallet with transaction A, probably because of looping over in-memory wallets, but the wallet didn't exist at the time of transaction A's creation

I believe this is how it happened for me, given the situation and the exact assert that failed (copied from my Discord post):

active_transactions::block_cemented_callback creates a wallet transaction then calls node::receive_confirmed with it, which then iterates over the wallets in memory via wallets::get_wallets and calls wallet_store::exists with the transaction and the wallet store including the MDB_dbi, which then calls wallet_store::begin, which then calls mdb_iterator::mdb_iterator, which fails because the wallet db didn't exist when the transaction was created, which leads to the failed assert in the crash

and I should note that this race is made more likely by the mutex which protects both wallets::create and wallets::get_wallets, meaning that the get_wallets call waits for the wallet to finish being created, making this race significantly more likely

Environment:

Local beta net release build of V22.0DB8

logs

Ends with:

[2020-Nov-01 11:26:41.953555]: 0x5602b9060210 {"action":"wallet_create"}
[2020-Nov-01 11:26:42.047889]: RPC request 0x5602b9060210 completed in: 94357 microseconds
@wezrule
Copy link
Contributor

wezrule commented Nov 17, 2020

Thanks for the detailed bug report. I also agree with your sequence of events, and have provided a fix for it here: #3051

@zhyatt
Copy link
Collaborator

zhyatt commented Sep 16, 2021

Closed by #3051

@zhyatt zhyatt closed this as completed Sep 16, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants