Merge pull request #26 from AntelopeIO/appbase_gh_25

Ensure that `plugin_shutdown()` is always called.

include/appbase/application_base.hpp

@@ -280,11 +280,11 @@ class application_base {
  * the application can call shutdown in the reverse order.
  */
  ///@{
- void plugin_initialized(abstract_plugin& plug) {
- initialized_plugins.push_back(&plug);
+ void plugin_initialized(abstract_plugin* plug) {
+ initialized_plugins.push_back(plug);
  }
- void plugin_started(abstract_plugin& plug) {
- running_plugins.push_back(&plug);
+ void plugin_started(abstract_plugin* plug) {
+ running_plugins.push_back(plug);
  }
  ///@}
 

include/appbase/application_instance.hpp

@@ -28,7 +28,7 @@ class plugin : public abstract_plugin {
  static_cast<Impl*>(this)->plugin_requires([&](auto& plug) { plug.initialize(options); });
  static_cast<Impl*>(this)->plugin_initialize(options);
  // ilog( "initializing plugin ${name}", ("name",name()) );
- app().plugin_initialized(*this);
+ app().plugin_initialized(this);
  }
  assert(_state == initialized); /// if initial state was not registered, final state cannot be initialized
  }
@@ -39,8 +39,8 @@ class plugin : public abstract_plugin {
  if (_state == initialized) {
  _state = started;
  static_cast<Impl*>(this)->plugin_requires([&](auto& plug) { plug.startup(); });
+ app().plugin_started(this); // add to `running_plugins` before so it will be shutdown if we throw in `plugin_startup()`
  static_cast<Impl*>(this)->plugin_startup();
- app().plugin_started(*this);
  }
  assert(_state == started); // if initial state was not initialized, final state cannot be started
  }

tests/basic_test.cpp

@@ -27,18 +27,25 @@ class pluginA : public appbase::plugin<pluginA>
  ("readonly", "open db in read only mode")
  ("dbsize", bpo::value<uint64_t>()->default_value( 8*1024 ), "Minimum size MB of database shared memory file")
  ("replay", "clear db and replay all blocks" )
+ ("throw_during_startup", "throw an exception in plugin_startup()" )
  ("log", "log messages" );
  }
 
  void plugin_initialize( const variables_map& options ) {
  readonly_ = !!options.count("readonly");
  replay_ = !!options.count("replay");
  log_ = !!options.count("log");
+ throw_during_startup_ = !!options.count("throw_during_startup");
  dbsize_ = options.at("dbsize").as<uint64_t>();
  log("initialize pluginA");
  }
 
- void plugin_startup() { log("starting pluginA"); }
+ void plugin_startup() {
+ log("starting pluginA");
+ if (throw_during_startup_)
+ do_throw("throwing as requested");
+ }
+
  void plugin_shutdown() {
  log("shutdown pluginA");
  if (shutdown_counter)
@@ -48,7 +55,7 @@ class pluginA : public appbase::plugin<pluginA>
  uint64_t dbsize() const { return dbsize_; }
  bool readonly() const { return readonly_; }
 
- void do_throw(std::string msg) { throw std::runtime_error(msg); }
+ void do_throw(const std::string& msg) { throw std::runtime_error(msg); }
  void set_shutdown_counter(uint32_t &c) { shutdown_counter = &c; }
 
  void log(std::string_view s) const {
@@ -59,6 +66,7 @@ class pluginA : public appbase::plugin<pluginA>
 private:
  bool readonly_ {false};
  bool replay_ {false};
+ bool throw_during_startup_ {false};
  bool log_ {false};
  uint64_t dbsize_ {0};
  uint32_t* shutdown_counter { nullptr };
@@ -327,6 +335,40 @@ BOOST_AUTO_TEST_CASE(exception_in_shutdown)
  // even though there was a throw
 }
 
+// -----------------------------------------------------------------------------
+// Here we make sure that if a plugin throws during `plugin_startup()`
+// 1. the exception is caught by the appbase framework, and logged
+// 2. all plugins are shutdown (verified with shutdown_counter)
+// -----------------------------------------------------------------------------
+BOOST_AUTO_TEST_CASE(exception_in_startup)
+{
+ appbase::application::register_plugin<pluginB>();
+
+ appbase::scoped_app app;
+
+ const char* argv[] = { bu::framework::current_test_case().p_name->c_str(),
+ "--plugin", "pluginA", "--log", "--throw_during_startup",
+ "--plugin", "pluginB", "--log2" };
+
+ BOOST_CHECK(app->initialize<pluginB>(sizeof(argv) / sizeof(char*), const_cast<char**>(argv)));
+
+ std::thread app_thread( [&]() {
+ auto& pA = app->get_plugin<pluginA>();
+ uint32_t shutdown_counter(0);
+ pA.set_shutdown_counter(shutdown_counter);
+
+ try {
+ app->startup();
+ } catch(const std::exception& e ) {
+ std::cout << "exception during startup (as expected): " << e.what() << "\n";
+ }
+ BOOST_CHECK(shutdown_counter == 1); // check that plugin_shutdown() was executed for pA
+ } );
+
+ app_thread.join();
+}
+
+
 // -----------------------------------------------------------------------------
 // Make sure that queue is emptied when `app->quit()` is called, and that the
 // queued tasks are *not* executed