Encapsulate the open wallets map and test wallet RPC thread safety (#5127) * Encapsulate the open wallets map behind synchronized accessors RPC handlers, the CLI and the Qt wallet accessed wallets::items without holding the mutex, racing with reload() on the receivable scan thread. The map is now private; callers go through open() or the new snapshot queries: all_wallets(), wallet_ids() and wallet_count(). * Test wallet RPC thread safety with concurrent requests Adds a wait_responses test primitive that keeps a whole batch of RPC requests in flight at once, together with an rpc_options override for the RPC → IPC connection count, which defaults to 1 on the dev network and would otherwise process requests one at a time. wallets::destroy now returns whether a wallet was destroyed instead of asserting on a missing wallet, so concurrent wallet_destroy requests for the same wallet are safe and exactly one of them wins. * Encapsulate `wallet_store::mutex` to fix lock order inversion TSAN reported a lock order inversion between wallets::mutex and wallet_store::mutex. wallets::create holds the former while constructing a wallet_store that takes the latter, but node.unlock_search evaluated system::wallet, which now takes wallets::mutex, while already holding a store mutex. That guard protected nothing: nano::fan is internally synchronized, so the value_set it wrapped was already safe. The test, node.send_unkeyed and the Qt lock button were all open-coding wallet::lock, which does the same thing and also refreshes the rep keys cache; they now call it. wallet_store::mutex is private, leaving only the compound operations inside wallet_store that genuinely need it. refresh_rep_keys_cache drops its store lock as well, since fetch revalidates the password itself and the lock only kept the diagnostic log line in step with the result. * Derive the locked state in `refresh_rep_keys_cache` from the fetch The cache checked valid_password and then fetched the private key as two separate operations. Holding the store mutex across the pair was the only thing keeping them in agreement, and it did not even cover wallet::lock, which writes the password fan without taking that mutex. fetch already reports the locked state through wallet_locked, so a single call yields both the key and the reason it was unavailable, and cannot disagree with itself under a concurrent rekey. Watch-only representatives still fall out as bad_private_key and are left uncached without warning.
__________
Type: commit
Hash: 901fdac25909df7d885e27d92362e52bcc57a68d
Created: yesterday
__________
gr0v1ty/nano-node:901fdac25909df7d885e27d92362e52bcc57a68d
| Test Case | Status | Duration | Median Deviation | Log | Graph |
|---|---|---|---|---|---|
| 5n4pr_conf_10k_bintree | PASS | 116 s | 2.0 s | - | Show |
| 5n4pr_conf_10k_change | PASS | 201 s | 41.0 s | - | Show |
| 5n4pr_conf_change_dependant | PASS | 111 s | -3.0 s | - | - |
| 5n4pr_conf_change_independant | PASS | 109 s | -5.0 s | - | - |
| 5n4pr_conf_send_dependant | PASS | 137 s | 21.0 s | - | - |
| 5n4pr_conf_send_independant | PASS | 106 s | -7.0 s | - | - |
| 5n4pr_rocks_10k_bintree | PASS | 107 s | -13.0 s | - | - |
| 5n4pr_rocks_10k_change | PASS | 157 s | -18.0 s | - | - |