From 49acfb3bd6267c3a3da990651f09beae8213e6a7 Mon Sep 17 00:00:00 2001 From: Eric Frias Date: Thu, 25 Jun 2015 10:33:39 -0400 Subject: [PATCH] Fix up application_impl's has_item() which was throwing to indicate a missing item instead of returning false. #55 --- libraries/app/application.cpp | 31 +++++++++++++++------- libraries/chain/block_database.cpp | 16 +++++++----- libraries/net/node.cpp | 42 ++++++++++++++++++++++++++++-- 3 files changed, 71 insertions(+), 18 deletions(-) diff --git a/libraries/app/application.cpp b/libraries/app/application.cpp index 769dbf2e..1c1a2f41 100644 --- a/libraries/app/application.cpp +++ b/libraries/app/application.cpp @@ -195,16 +195,27 @@ namespace detail { * If delegate has the item, the network has no need to fetch it. */ virtual bool has_item( const net::item_id& id ) override - { try { - if( id.item_type == graphene::net::block_message_type ) - { - return _chain_db->is_known_block( id.item_hash ); - } - else - { - return _chain_db->is_known_transaction( id.item_hash ); - } - } FC_CAPTURE_AND_RETHROW( (id) ) } + { + try + { + if( id.item_type == graphene::net::block_message_type ) + { + // for some reason, the contains() function called by is_known_block + // throws when the block is not present (instead of returning false) + try + { + return _chain_db->is_known_block( id.item_hash ); + } + catch (const fc::key_not_found_exception&) + { + return false; + } + } + else + return _chain_db->is_known_transaction( id.item_hash ); // is_known_transaction behaves normally + } + FC_CAPTURE_AND_RETHROW( (id) ) + } /** * @brief allows the application to validate an item prior to broadcasting to peers. diff --git a/libraries/chain/block_database.cpp b/libraries/chain/block_database.cpp index 9bc95e49..434c6682 100644 --- a/libraries/chain/block_database.cpp +++ b/libraries/chain/block_database.cpp @@ -88,7 +88,8 @@ void block_database::remove( const block_id_type& id ) index_entry e; auto index_pos = sizeof(e)*block_header::num_from_id(id); _block_num_to_pos.seekg( 0, _block_num_to_pos.end ); - FC_ASSERT( _block_num_to_pos.tellg() > index_pos ); + if ( _block_num_to_pos.tellg() <= index_pos ) + FC_THROW_EXCEPTION(fc::key_not_found_exception, "Block ${id} not contained in block database", ("id", id)); _block_num_to_pos.seekg( index_pos ); _block_num_to_pos.read( (char*)&e, sizeof(e) ); @@ -110,8 +111,8 @@ bool block_database::contains( const block_id_type& id )const index_entry e; auto index_pos = sizeof(e)*block_header::num_from_id(id); _block_num_to_pos.seekg( 0, _block_num_to_pos.end ); - FC_ASSERT( _block_num_to_pos.tellg() > index_pos ); - + if ( _block_num_to_pos.tellg() <= index_pos ) + FC_THROW_EXCEPTION(fc::key_not_found_exception, "Block ${id} not contained in block database", ("id", id)); _block_num_to_pos.seekg( index_pos ); _block_num_to_pos.read( (char*)&e, sizeof(e) ); @@ -124,7 +125,8 @@ block_id_type block_database::fetch_block_id( uint32_t block_num )const index_entry e; auto index_pos = sizeof(e)*block_num; _block_num_to_pos.seekg( 0, _block_num_to_pos.end ); - FC_ASSERT( _block_num_to_pos.tellg() > index_pos ); + if ( _block_num_to_pos.tellg() <= index_pos ) + FC_THROW_EXCEPTION(fc::key_not_found_exception, "Block number ${block_num} not contained in block database", ("block_num", block_num)); _block_num_to_pos.seekg( index_pos ); _block_num_to_pos.read( (char*)&e, sizeof(e) ); @@ -140,7 +142,8 @@ optional block_database::fetch_optional( const block_id_type& id ) index_entry e; auto index_pos = sizeof(e)*block_header::num_from_id(id); _block_num_to_pos.seekg( 0, _block_num_to_pos.end ); - FC_ASSERT( _block_num_to_pos.tellg() > index_pos ); + if ( _block_num_to_pos.tellg() <= index_pos ) + FC_THROW_EXCEPTION(fc::key_not_found_exception, "Block ${id} not contained in block database", ("id", id)); _block_num_to_pos.seekg( index_pos ); _block_num_to_pos.read( (char*)&e, sizeof(e) ); @@ -170,7 +173,8 @@ optional block_database::fetch_by_number( uint32_t block_num )cons index_entry e; auto index_pos = sizeof(e)*block_num; _block_num_to_pos.seekg( 0, _block_num_to_pos.end ); - FC_ASSERT( _block_num_to_pos.tellg() > index_pos ); + if ( _block_num_to_pos.tellg() <= index_pos ) + FC_THROW_EXCEPTION(fc::key_not_found_exception, "Block number ${block_num} not contained in block database", ("block_num", block_num)); _block_num_to_pos.seekg( index_pos, _block_num_to_pos.beg ); _block_num_to_pos.read( (char*)&e, sizeof(e) ); diff --git a/libraries/net/node.cpp b/libraries/net/node.cpp index 45aa03c3..0e17e4c5 100644 --- a/libraries/net/node.cpp +++ b/libraries/net/node.cpp @@ -1525,7 +1525,7 @@ namespace graphene { namespace net { namespace detail { else dlog("delayed_peer_deletion_task is already scheduled (current size of _peers_to_delete is ${size})", ("size", number_of_peers_to_delete)); #else - dlog("scheduling peer for deletion: ${peer} (this will not block)"); + dlog("scheduling peer for deletion: ${peer} (this will not block)", ("peer", peer_to_delete->get_remote_endpoint())); _peers_to_delete.push_back(peer_to_delete); if (!_node_is_shutting_down && (!_delayed_peer_deletion_task_done.valid() || _delayed_peer_deletion_task_done.ready())) @@ -5040,7 +5040,44 @@ namespace graphene { namespace net { namespace detail { return statistics; } -#define INVOKE_AND_COLLECT_STATISTICS(method_name, ...) \ +// define VERBOSE_NODE_DELEGATE_LOGGING to log whenever the node delegate throws exceptions +//#define VERBOSE_NODE_DELEGATE_LOGGING +#ifdef VERBOSE_NODE_DELEGATE_LOGGING +# define INVOKE_AND_COLLECT_STATISTICS(method_name, ...) \ + try \ + { \ + call_statistics_collector statistics_collector(#method_name, \ + &_ ## method_name ## _execution_accumulator, \ + &_ ## method_name ## _delay_before_accumulator, \ + &_ ## method_name ## _delay_after_accumulator); \ + if (_thread->is_current()) \ + { \ + call_statistics_collector::actual_execution_measurement_helper helper(statistics_collector); \ + return _node_delegate->method_name(__VA_ARGS__); \ + } \ + else \ + return _thread->async([&](){ \ + call_statistics_collector::actual_execution_measurement_helper helper(statistics_collector); \ + return _node_delegate->method_name(__VA_ARGS__); \ + }, "invoke " BOOST_STRINGIZE(method_name)).wait(); \ + } \ + catch (const fc::exception& e) \ + { \ + dlog("node_delegate threw fc::exception: ${e}", ("e", e)); \ + throw; \ + } \ + catch (const std::exception& e) \ + { \ + dlog("node_delegate threw std::exception: ${e}", ("e", e.what())); \ + throw; \ + } \ + catch (...) \ + { \ + dlog("node_delegate threw unrecognized exception"); \ + throw; \ + } +#else +# define INVOKE_AND_COLLECT_STATISTICS(method_name, ...) \ call_statistics_collector statistics_collector(#method_name, \ &_ ## method_name ## _execution_accumulator, \ &_ ## method_name ## _delay_before_accumulator, \ @@ -5055,6 +5092,7 @@ namespace graphene { namespace net { namespace detail { call_statistics_collector::actual_execution_measurement_helper helper(statistics_collector); \ return _node_delegate->method_name(__VA_ARGS__); \ }, "invoke " BOOST_STRINGIZE(method_name)).wait() +#endif bool statistics_gathering_node_delegate_wrapper::has_item( const net::item_id& id ) {