From f9845f06e67123f6bbbe2b1b8f7330bbb0a1cb9c Mon Sep 17 00:00:00 2001 From: Guilherme Lawless Date: Tue, 31 Mar 2020 12:24:26 +0100 Subject: [PATCH] Wrapper for RPC worker tasks (#2692) * Wrapper for RPC worker tasks * Pass the rpc ptr by const ref (Wes review) --- nano/nano_node/entry.cpp | 4 +- nano/node/json_handler.cpp | 108 ++++++++++++++++++------------------- nano/node/json_handler.hpp | 1 + nano/rpc_test/rpc.cpp | 23 ++++++++ 4 files changed, 80 insertions(+), 56 deletions(-) diff --git a/nano/nano_node/entry.cpp b/nano/nano_node/entry.cpp index b3b19ec5ab..d95c3e6981 100644 --- a/nano/nano_node/entry.cpp +++ b/nano/nano_node/entry.cpp @@ -973,8 +973,8 @@ int main (int argc, char * const * argv) auto inactive_node_l = nano::default_inactive_node (data_path, vm); nano::node_rpc_config config; nano::ipc::ipc_server server (*inactive_node_l->node, config); - nano::json_handler handler_l (*inactive_node_l->node, config, command_l.str (), response_handler_l); - handler_l.process_request (); + auto handler_l (std::make_shared (*inactive_node_l->node, config, command_l.str (), response_handler_l)); + handler_l->process_request (); } else if (vm.count ("debug_validate_blocks")) { diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp index 336a023b42..4b89e6666b 100644 --- a/nano/node/json_handler.cpp +++ b/nano/node/json_handler.cpp @@ -35,6 +35,24 @@ node_rpc_config (node_rpc_config_a) { } +std::function nano::json_handler::create_worker_task (std::function const &)> const & action_a) +{ + return [rpc_l = shared_from_this (), action_a]() { + try + { + action_a (rpc_l); + } + catch (std::runtime_error const &) + { + json_error_response (rpc_l->response, "Unable to parse JSON"); + } + catch (...) + { + json_error_response (rpc_l->response, "Internal server error in RPC"); + } + }; +} + void nano::json_handler::process_request (bool unsafe_a) { try @@ -487,8 +505,7 @@ void nano::json_handler::account_block_count () void nano::json_handler::account_create () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); if (!rpc_l->ec) { @@ -525,7 +542,7 @@ void nano::json_handler::account_create () } } rpc_l->response_errors (); - }); + })); } void nano::json_handler::account_get () @@ -620,8 +637,7 @@ void nano::json_handler::account_list () void nano::json_handler::account_move () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); if (!rpc_l->ec) { @@ -655,13 +671,12 @@ void nano::json_handler::account_move () } } rpc_l->response_errors (); - }); + })); } void nano::json_handler::account_remove () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); auto account (rpc_l->account_impl ()); if (!rpc_l->ec) @@ -676,7 +691,7 @@ void nano::json_handler::account_remove () } } rpc_l->response_errors (); - }); + })); } void nano::json_handler::account_representative () @@ -696,8 +711,7 @@ void nano::json_handler::account_representative () void nano::json_handler::account_representative_set () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l, work_generation_enabled = node.work_generation_enabled ()]() { + node.worker.push_task (create_worker_task ([work_generation_enabled = node.work_generation_enabled ()](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); auto account (rpc_l->account_impl ()); std::string representative_text (rpc_l->request.get ("representative")); @@ -758,7 +772,7 @@ void nano::json_handler::account_representative_set () { rpc_l->response_errors (); } - }); + })); } void nano::json_handler::account_weight () @@ -793,8 +807,7 @@ void nano::json_handler::accounts_balances () void nano::json_handler::accounts_create () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); auto count (rpc_l->count_impl ()); if (!rpc_l->ec) @@ -814,7 +827,7 @@ void nano::json_handler::accounts_create () rpc_l->response_l.add_child ("accounts", accounts); } rpc_l->response_errors (); - }); + })); } void nano::json_handler::accounts_frontiers () @@ -2829,8 +2842,7 @@ void nano::json_handler::node_id_delete () void nano::json_handler::password_change () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); if (!rpc_l->ec) { @@ -2848,13 +2860,12 @@ void nano::json_handler::password_change () } } rpc_l->response_errors (); - }); + })); } void nano::json_handler::password_enter () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); if (!rpc_l->ec) { @@ -2864,7 +2875,7 @@ void nano::json_handler::password_enter () rpc_l->response_l.put ("valid", error ? "0" : "1"); } rpc_l->response_errors (); - }); + })); } void nano::json_handler::password_valid (bool wallet_locked) @@ -3027,8 +3038,7 @@ void nano::json_handler::pending_exists () void nano::json_handler::payment_begin () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { std::string id_text (rpc_l->request.get ("wallet")); nano::wallet_id id; if (!id.decode_hex (id_text)) @@ -3094,13 +3104,12 @@ void nano::json_handler::payment_begin () rpc_l->ec = nano::error_common::bad_wallet_number; } rpc_l->response_errors (); - }); + })); } void nano::json_handler::payment_init () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); if (!rpc_l->ec) { @@ -3117,7 +3126,7 @@ void nano::json_handler::payment_init () } } rpc_l->response_errors (); - }); + })); } void nano::json_handler::payment_end () @@ -3176,8 +3185,7 @@ void nano::json_handler::payment_wait () void nano::json_handler::process () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { const bool json_block_l = rpc_l->request.get ("json_block", false); const bool watch_work_l = rpc_l->request.get ("watch_work", true); std::shared_ptr block; @@ -3343,7 +3351,7 @@ void nano::json_handler::process () } } rpc_l->response_errors (); - }); + })); } void nano::json_handler::receive () @@ -4125,14 +4133,13 @@ void nano::json_handler::unchecked () void nano::json_handler::unchecked_clear () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto transaction (rpc_l->node.store.tx_begin_write ({ tables::unchecked })); rpc_l->node.store.unchecked_clear (transaction); rpc_l->node.ledger.cache.unchecked_count = 0; rpc_l->response_l.put ("success", ""); rpc_l->response_errors (); - }); + })); } void nano::json_handler::unchecked_get () @@ -4306,8 +4313,7 @@ void nano::json_handler::validate_account_number () void nano::json_handler::wallet_add () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); if (!rpc_l->ec) { @@ -4332,13 +4338,12 @@ void nano::json_handler::wallet_add () } } rpc_l->response_errors (); - }); + })); } void nano::json_handler::wallet_add_watch () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); if (!rpc_l->ec) { @@ -4367,7 +4372,7 @@ void nano::json_handler::wallet_add_watch () } } rpc_l->response_errors (); - }); + })); } void nano::json_handler::wallet_info () @@ -4438,8 +4443,7 @@ void nano::json_handler::wallet_balances () void nano::json_handler::wallet_change_seed () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); if (!rpc_l->ec) { @@ -4469,7 +4473,7 @@ void nano::json_handler::wallet_change_seed () } } rpc_l->response_errors (); - }); + })); } void nano::json_handler::wallet_contains () @@ -4487,8 +4491,7 @@ void nano::json_handler::wallet_contains () void nano::json_handler::wallet_create () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { nano::raw_key seed; auto seed_text (rpc_l->request.get_optional ("seed")); if (seed_text.is_initialized () && seed.data.decode_hex (seed_text.get ())) @@ -4519,13 +4522,12 @@ void nano::json_handler::wallet_create () } } rpc_l->response_errors (); - }); + })); } void nano::json_handler::wallet_destroy () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { std::string wallet_text (rpc_l->request.get ("wallet")); nano::wallet_id wallet; if (!wallet.decode_hex (wallet_text)) @@ -4547,7 +4549,7 @@ void nano::json_handler::wallet_destroy () rpc_l->ec = nano::error_common::bad_wallet_number; } rpc_l->response_errors (); - }); + })); } void nano::json_handler::wallet_export () @@ -4808,8 +4810,7 @@ void nano::json_handler::wallet_representative () void nano::json_handler::wallet_representative_set () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); std::string representative_text (rpc_l->request.get ("representative")); auto representative (rpc_l->account_impl (representative_text, nano::error_rpc::bad_representative_number)); @@ -4856,7 +4857,7 @@ void nano::json_handler::wallet_representative_set () } } rpc_l->response_errors (); - }); + })); } void nano::json_handler::wallet_republish () @@ -5053,8 +5054,7 @@ void nano::json_handler::work_get () void nano::json_handler::work_set () { - auto rpc_l (shared_from_this ()); - node.worker.push_task ([rpc_l]() { + node.worker.push_task (create_worker_task ([](std::shared_ptr const & rpc_l) { auto wallet (rpc_l->wallet_impl ()); auto account (rpc_l->account_impl ()); auto work (rpc_l->work_optional_impl ()); @@ -5069,7 +5069,7 @@ void nano::json_handler::work_set () } } rpc_l->response_errors (); - }); + })); } void nano::json_handler::work_validate () diff --git a/nano/node/json_handler.hpp b/nano/node/json_handler.hpp index f8badbabaa..2cd970d79e 100644 --- a/nano/node/json_handler.hpp +++ b/nano/node/json_handler.hpp @@ -167,6 +167,7 @@ class json_handler : public std::enable_shared_from_this bool enable_sign_hash{ false }; std::function stop_callback; nano::node_rpc_config const & node_rpc_config; + std::function create_worker_task (std::function const &)> const &); }; class inprocess_rpc_handler final : public nano::rpc_handler_interface diff --git a/nano/rpc_test/rpc.cpp b/nano/rpc_test/rpc.cpp index acfb6d4763..7e92190ec8 100644 --- a/nano/rpc_test/rpc.cpp +++ b/nano/rpc_test/rpc.cpp @@ -154,6 +154,29 @@ class scoped_io_thread_name_change }; } +TEST (rpc, wrapped_task) +{ + nano::system system; + auto & node = *add_ipc_enabled_node (system); + nano::node_rpc_config node_rpc_config; + std::atomic response (false); + auto response_handler_l ([&response](std::string const & response_a) { + std::stringstream istream (response_a); + boost::property_tree::ptree json_l; + ASSERT_NO_THROW (boost::property_tree::read_json (istream, json_l)); + ASSERT_EQ (1, json_l.count ("error")); + ASSERT_EQ ("Unable to parse JSON", json_l.get ("error")); + response = true; + }); + auto handler_l (std::make_shared (node, node_rpc_config, "", response_handler_l)); + auto task (handler_l->create_worker_task ([](std::shared_ptr) { + // Exception should get caught + throw std::runtime_error (""); + })); + system.nodes[0]->worker.push_task (task); + ASSERT_TIMELY (5s, response == true); +} + TEST (rpc, account_balance) { nano::system system;