From 4b1c0d69f4abe5c25303f2f0d07bcf6a7daf2ff9 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Thu, 21 Apr 2016 00:11:11 +0100 Subject: [PATCH] simplewallet: some background refresh threading fixes We want to lock operations which access the blockchain in wallet2. We also want the background refresh to happen again when we cancel a foreground refresh. Wrap the locking setup in a macro so it doesn't get copy/pasted/mangled, and use a scope exit trick to ensure it's always properly restored. --- src/simplewallet/simplewallet.cpp | 55 ++++++++++++++++++++++++------- src/simplewallet/simplewallet.h | 1 + 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index 29c92e049..280c4e276 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -77,6 +77,19 @@ typedef cryptonote::simple_wallet sw; #define DEFAULT_MIX 4 +#define LOCK_REFRESH_THREAD_SCOPE() \ + bool auto_refresh_run = m_auto_refresh_run.load(std::memory_order_relaxed); \ + m_auto_refresh_run.store(false, std::memory_order_relaxed); \ + /* stop any background refresh, and take over */ \ + m_wallet->stop(); \ + boost::unique_lock lock(m_auto_refresh_mutex); \ + m_auto_refresh_cond.notify_one(); \ + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = epee::misc_utils::create_scope_leave_handler([&](){ \ + m_auto_refresh_run.store(auto_refresh_run, std::memory_order_relaxed); \ + if (auto_refresh_run) \ + m_auto_refresh_thread = boost::thread([&]{wallet_refresh_thread();}); \ + }) + namespace { const command_line::arg_descriptor arg_wallet_file = {"wallet-file", sw::tr("Use wallet "), ""}; @@ -1584,6 +1597,7 @@ bool simple_wallet::save(const std::vector &args) { try { + LOCK_REFRESH_THREAD_SCOPE(); m_wallet->store(); success_msg_writer() << tr("Wallet data saved"); } @@ -1752,12 +1766,7 @@ bool simple_wallet::refresh_main(uint64_t start_height, bool reset) if (!try_connect_to_daemon()) return true; - bool auto_refresh_run = m_auto_refresh_run.load(std::memory_order_relaxed); - m_auto_refresh_run.store(false, std::memory_order_relaxed); - // stop any background refresh, and take over - m_wallet->stop(); - boost::unique_lock lock(m_auto_refresh_mutex); - m_auto_refresh_cond.notify_one(); + LOCK_REFRESH_THREAD_SCOPE(); if (reset) m_wallet->rescan_blockchain(false); @@ -1770,13 +1779,13 @@ bool simple_wallet::refresh_main(uint64_t start_height, bool reset) try { m_in_manual_refresh.store(true, std::memory_order_relaxed); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = epee::misc_utils::create_scope_leave_handler([&](){m_in_manual_refresh.store(false, std::memory_order_relaxed);}); m_wallet->refresh(start_height, fetched_blocks); - m_in_manual_refresh.store(false, std::memory_order_relaxed); ok = true; // Clear line "Height xxx of xxx" std::cout << "\r \r"; success_msg_writer(true) << tr("Refresh done, blocks received: ") << fetched_blocks; - show_balance(); + show_balance_unlocked(); } catch (const tools::error::daemon_busy&) { @@ -1817,8 +1826,6 @@ bool simple_wallet::refresh_main(uint64_t start_height, bool reset) fail_msg_writer() << tr("refresh failed: ") << ss.str() << ". " << tr("Blocks received: ") << fetched_blocks; } - m_in_manual_refresh.store(false, std::memory_order_relaxed); - m_auto_refresh_run.store(auto_refresh_run, std::memory_order_relaxed); return true; } //---------------------------------------------------------------------------------------------------- @@ -1838,15 +1845,24 @@ bool simple_wallet::refresh(const std::vector& args) return refresh_main(start_height, false); } //---------------------------------------------------------------------------------------------------- -bool simple_wallet::show_balance(const std::vector& args/* = std::vector()*/) +bool simple_wallet::show_balance_unlocked() { success_msg_writer() << tr("Balance: ") << print_money(m_wallet->balance()) << ", " << tr("unlocked balance: ") << print_money(m_wallet->unlocked_balance()); return true; } //---------------------------------------------------------------------------------------------------- +bool simple_wallet::show_balance(const std::vector& args/* = std::vector()*/) +{ + LOCK_REFRESH_THREAD_SCOPE(); + show_balance_unlocked(); + return true; +} +//---------------------------------------------------------------------------------------------------- bool simple_wallet::show_incoming_transfers(const std::vector& args) { + LOCK_REFRESH_THREAD_SCOPE(); + bool filter = false; bool available = false; if (!args.empty()) @@ -1912,6 +1928,8 @@ bool simple_wallet::show_payments(const std::vector &args) return true; } + LOCK_REFRESH_THREAD_SCOPE(); + message_writer() << boost::format("%68s%68s%12s%21s%16s") % tr("payment") % tr("transaction") % tr("height") % tr("amount") % tr("unlock time"); @@ -1989,6 +2007,7 @@ bool simple_wallet::rescan_spent(const std::vector &args) try { + LOCK_REFRESH_THREAD_SCOPE(); m_wallet->rescan_spent(); } catch (const tools::error::daemon_busy&) @@ -2027,6 +2046,8 @@ bool simple_wallet::transfer_main(bool new_algorithm, const std::vector local_args = args_; size_t fake_outs_count; @@ -2346,6 +2367,7 @@ bool simple_wallet::sweep_unmixable(const std::vector &args_) return true; } + LOCK_REFRESH_THREAD_SCOPE(); try { // figure out what tx will be necessary @@ -2503,6 +2525,8 @@ bool simple_wallet::get_tx_key(const std::vector &args_) } crypto::hash txid = *reinterpret_cast(txid_data.data()); + LOCK_REFRESH_THREAD_SCOPE(); + crypto::secret_key tx_key; bool r = m_wallet->get_tx_key(txid, tx_key); if (r) @@ -2537,6 +2561,8 @@ bool simple_wallet::check_tx_key(const std::vector &args_) } crypto::hash txid = *reinterpret_cast(txid_data.data()); + LOCK_REFRESH_THREAD_SCOPE(); + cryptonote::blobdata tx_key_data; if(!epee::string_tools::parse_hexstr_to_binbuff(local_args[1], tx_key_data)) { @@ -2641,6 +2667,8 @@ bool simple_wallet::show_transfers(const std::vector &args_) return true; } + LOCK_REFRESH_THREAD_SCOPE(); + // optional in/out selector if (local_args.size() > 0) { if (local_args[0] == "in" || local_args[0] == "incoming") { @@ -2795,7 +2823,12 @@ bool simple_wallet::run() void simple_wallet::stop() { m_cmd_binder.stop_handling(); + + m_auto_refresh_run.store(false, std::memory_order_relaxed); m_wallet->stop(); + // make the background refresh thread quit + boost::unique_lock lock(m_auto_refresh_mutex); + m_auto_refresh_cond.notify_one(); } //---------------------------------------------------------------------------------------------------- bool simple_wallet::print_address(const std::vector &args/* = std::vector()*/) diff --git a/src/simplewallet/simplewallet.h b/src/simplewallet/simplewallet.h index 21bbfa566..72ab4d29f 100644 --- a/src/simplewallet/simplewallet.h +++ b/src/simplewallet/simplewallet.h @@ -114,6 +114,7 @@ namespace cryptonote bool stop_mining(const std::vector &args); bool save_bc(const std::vector& args); bool refresh(const std::vector &args); + bool show_balance_unlocked(); bool show_balance(const std::vector &args = std::vector()); bool show_incoming_transfers(const std::vector &args); bool show_payments(const std::vector &args);