Merge pull request #976 from AntelopeIO/GH-946-emit-exception-test

Handle exceptions in controller signals

libraries/chain/controller.cpp

@@ -928,6 +928,38 @@ struct controller_impl {
              // Read-write tasks are not being executed.
    };
 
+   /**
+    *  Plugins / observers listening to signals emitted might trigger
+    *  errors and throw exceptions. Unless those exceptions are caught it could impact consensus and/or
+    *  cause a node to fork.
+    *
+    *  Initiate shutdown and rethrow controller_emit_signal_exception transactions as these exeception are
+    *  critical errors where a node should abort the current block and shutdown.
+    */
+   template<typename Signal, typename Arg>
+   void emit( const Signal& s, Arg&& a, const char* file, uint32_t line ) {
+      try {
+         s( std::forward<Arg>( a ));
+      } catch (std::bad_alloc& e) {
+         wlog( "${f}:${l} std::bad_alloc: ${w}", ("f", file)("l", line)("w", e.what()) );
+         throw e;
+      } catch (boost::interprocess::bad_alloc& e) {
+         wlog( "${f}:${l} boost::interprocess::bad alloc: ${w}", ("f", file)("l", line)("w", e.what()) );
+         throw e;
+      } catch ( controller_emit_signal_exception& e ) {
+         wlog( "${f}:${l} controller_emit_signal_exception: ${details}", ("f", file)("l", line)("details", e.to_detail_string()) );
+         wlog( "Shutting down due to controller_emit_signal_exception" );
+         shutdown();
+         throw e;
+      } catch ( fc::exception& e ) {
+         wlog( "${f}:${l} fc::exception: ${details}", ("f", file)("l", line)("details", e.to_detail_string()) );
+      } catch ( std::exception& e ) {
+         wlog( "std::exception: ${details}", ("f", file)("l", line)("details", e.what()) );
+      } catch ( ... ) {
+         wlog( "${f}:${l} signal handler threw exception", ("f", file)("l", line) );
+      }
+   }
+
 #if LLVM_VERSION_MAJOR < 9
    // LLVM versions prior to 9 do a set_new_handler() in a static global initializer. Reset LLVM's custom handler
    // back to the default behavior of throwing bad_alloc so we can possibly exit cleanly and not just abort as LLVM's
@@ -998,7 +1030,9 @@ struct controller_impl {
    std::function<void(speculative_block_metrics)> _update_speculative_block_metrics;
    std::function<void(incoming_block_metrics)> _update_incoming_block_metrics;
 
-   vote_processor_t vote_processor{aggregated_vote,
+   vote_processor_t vote_processor{[this](const vote_signal_params& p) {
+                                      emit(aggregated_vote, p, __FILE__, __LINE__);
+                                   },
                                    [this](const block_id_type& id) -> block_state_ptr {
                                       return fork_db_.apply_s<block_state_ptr>([&](const auto& fork_db) {
                                          return fork_db.get_block(id);
@@ -3102,14 +3136,17 @@ struct controller_impl {
          } catch( const protocol_feature_bad_block_exception& ) {
             throw;
          } catch ( const std::bad_alloc& ) {
-           throw;
+            throw;
          } catch ( const boost::interprocess::bad_alloc& ) {
-           throw;
+            throw;
+         } catch ( const controller_emit_signal_exception& e ) {
+            wlog( "on block transaction failed due to controller_emit_signal_exception: ${e}", ("e", e.to_detail_string()) );
+            throw;
          } catch (const fc::exception& e) {
-           handle_exception(e);
+            handle_exception(e);
          } catch (const std::exception& e) {
-           auto wrapper = fc::std_exception_wrapper::from_current_exception(e);
-           handle_exception(wrapper);
+            auto wrapper = fc::std_exception_wrapper::from_current_exception(e);
+            handle_exception(wrapper);
          }
 
          // this code is hit if an exception was thrown, and handled by `handle_exception`
@@ -3290,6 +3327,9 @@ struct controller_impl {
          } catch( const boost::interprocess::bad_alloc& e ) {
             elog( "on block transaction failed due to a bad allocation" );
             throw;
+         } catch ( const controller_emit_signal_exception& e ) {
+            wlog( "on block transaction failed due to controller_emit_signal_exception: ${e}", ("e", e.to_detail_string()) );
+            throw;
          } catch( const fc::exception& e ) {
             wlog( "on block transaction failed, but shouldn't impact block generation, system contract needs update" );
             edump((e.to_detail_string()));
@@ -3410,7 +3450,8 @@ struct controller_impl {
             fork_db_.apply<void>(add_completed_block);
          }
 
-         chain_head = block_handle{cb.bsp};
+         // if an exception is thrown, reset chain_head to prior value
+         fc::scoped_set_value ch(chain_head, cb.bsp);
 
          if (s == controller::block_status::irreversible && replaying) {
             block_handle_accessor::apply_l<void>(chain_head, [&](const auto& head) {
@@ -3468,6 +3509,8 @@ struct controller_impl {
          }
 
          log_applied(s);
+
+         ch.dismiss(); // don't reset chain_head if no exception
       } catch (...) {
          // dont bother resetting pending, instead abort the block
          reset_pending_on_exit.cancel();

libraries/chain/include/eosio/chain/controller.hpp

@@ -500,35 +500,4 @@ namespace eosio::chain {
          std::unique_ptr<controller_impl> my;
    }; // controller
 
-   /**
-    *  Plugins / observers listening to signals emitted might trigger
-    *  errors and throw exceptions. Unless those exceptions are caught it could impact consensus and/or
-    *  cause a node to fork.
-    *
-    *  If it is ever desirable to let a signal handler bubble an exception out of this method
-    *  a full audit of its uses needs to be undertaken.
-    *
-    */
-   template<typename Signal, typename Arg>
-   void emit( const Signal& s, Arg&& a, const char* file, uint32_t line ) {
-      try {
-         s( std::forward<Arg>( a ));
-      } catch (std::bad_alloc& e) {
-         wlog( "${f}:${l} std::bad_alloc: ${w}", ("f", file)("l", line)("w", e.what()) );
-         throw e;
-      } catch (boost::interprocess::bad_alloc& e) {
-         wlog( "${f}:${l} boost::interprocess::bad alloc: ${w}", ("f", file)("l", line)("w", e.what()) );
-         throw e;
-      } catch ( controller_emit_signal_exception& e ) {
-         wlog( "${f}:${l} controller_emit_signal_exception: ${details}", ("f", file)("l", line)("details", e.to_detail_string()) );
-         throw e;
-      } catch ( fc::exception& e ) {
-         wlog( "${f}:${l} fc::exception: ${details}", ("f", file)("l", line)("details", e.to_detail_string()) );
-      } catch ( std::exception& e ) {
-         wlog( "std::exception: ${details}", ("f", file)("l", line)("details", e.what()) );
-      } catch ( ... ) {
-         wlog( "${f}:${l} signal handler threw exception", ("f", file)("l", line) );
-      }
-   }
-
 }  /// eosio::chain

libraries/chain/include/eosio/chain/vote_processor.hpp

@@ -45,9 +45,10 @@ class vote_processor_t {
       >
    >;
 
+   using emit_vote_signal_func_t = std::function<void(const vote_signal_params&)>;
    using fetch_block_func_t = std::function<block_state_ptr(const block_id_type&)>;
 
-   vote_signal_t&               vote_signal;
+   emit_vote_signal_func_t      emit_vote_signal_func;
    fetch_block_func_t           fetch_block_func;
 
    std::mutex                   mtx;
@@ -68,10 +69,8 @@ class vote_processor_t {
              const finalizer_authority_ptr& active_auth, const finalizer_authority_ptr& pending_auth) {
       if (connection_id != 0) { // this nodes vote was already signaled
          if (status != vote_result_t::duplicate) { // don't bother emitting duplicates
-            chain::emit( vote_signal,
-                         vote_signal_params{connection_id, status,
-                                            std::cref(msg), std::cref(active_auth), std::cref(pending_auth)},
-                         __FILE__, __LINE__ );
+            emit_vote_signal_func(vote_signal_params{connection_id, status,
+                                                     std::cref(msg), std::cref(active_auth), std::cref(pending_auth)});
          }
       }
    }
@@ -159,11 +158,12 @@ class vote_processor_t {
    }
 
 public:
-   explicit vote_processor_t(vote_signal_t& vote_signal, fetch_block_func_t&& get_block)
-      : vote_signal(vote_signal)
-      , fetch_block_func(get_block)
+   explicit vote_processor_t(emit_vote_signal_func_t&& emit_vote_signal, fetch_block_func_t&& get_block)
+      : emit_vote_signal_func(std::move(emit_vote_signal))
+      , fetch_block_func(std::move(get_block))
    {
-      assert(get_block);
+      assert(emit_vote_signal_func);
+      assert(fetch_block_func);
    }
 
    ~vote_processor_t() {

plugins/test_control_api_plugin/test_control_api_plugin.cpp

@@ -46,6 +46,11 @@ void test_control_api_plugin::plugin_startup() {
    app().get_plugin<http_plugin>().add_api({
       TEST_CONTROL_RW_CALL(kill_node_on_producer, 202, http_params_types::params_required)
    }, appbase::exec_queue::read_write);
+
+   app().get_plugin<http_plugin>().add_api({
+      TEST_CONTROL_RW_CALL(throw_on, 202, http_params_types::params_required)
+   }, appbase::exec_queue::read_write);
+
 }
 
 void test_control_api_plugin::plugin_shutdown() {}

plugins/test_control_plugin/include/eosio/test_control_plugin/test_control_plugin.hpp

@@ -27,6 +27,12 @@ class read_write {
       using kill_node_on_producer_results = empty;
       kill_node_on_producer_results kill_node_on_producer(const kill_node_on_producer_params& params) const;
 
+      struct throw_on_params {
+         std::string  signal;    // which signal handler to throw exception from
+         std::string  exception; // exception to throw
+      };
+      empty throw_on(const throw_on_params& params) const;
+
    private:
       test_control_ptr my;
 };
@@ -61,3 +67,4 @@ class test_control_plugin : public plugin<test_control_plugin> {
 
 FC_REFLECT(eosio::test_control_apis::empty, )
 FC_REFLECT(eosio::test_control_apis::read_write::kill_node_on_producer_params, (producer)(where_in_sequence)(based_on_lib) )
+FC_REFLECT(eosio::test_control_apis::read_write::throw_on_params, (signal)(exception) )