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