From ea84f3e8246bd46f19861f36c59d36141b34789a Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Fri, 26 May 2023 16:03:24 -0400
Subject: [PATCH 1/2] Consistent shutdown when `plugin_startup()` fails.

---
 include/appbase/application_instance.hpp |  5 ++++
 tests/basic_test.cpp                     | 36 ++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/include/appbase/application_instance.hpp b/include/appbase/application_instance.hpp
index a2430d9..218f6e5 100644
--- a/include/appbase/application_instance.hpp
+++ b/include/appbase/application_instance.hpp
@@ -41,6 +41,11 @@ class plugin : public abstract_plugin {
          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();
+         if (app().is_quiting()) {
+            // some plugins, instead of throwing an exception when failing to start, call app().quit().
+            // throw exception to ensure no more plugin gets initialized, and all running plugins receive `plugin_shutdown()`
+            throw std::runtime_error("plugin " + name() + " quit during startup");
+         }
       }
       assert(_state == started); // if initial state was not initialized, final state cannot be started
    }
diff --git a/tests/basic_test.cpp b/tests/basic_test.cpp
index 7bd3f0c..6debee7 100644
--- a/tests/basic_test.cpp
+++ b/tests/basic_test.cpp
@@ -28,6 +28,7 @@ class pluginA : public appbase::plugin<pluginA>
          ("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()" )
+         ("quit_during_startup", "calls app().quit() plugin_startup()" )
          ("log", "log messages" );
    }
 
@@ -36,6 +37,7 @@ class pluginA : public appbase::plugin<pluginA>
       replay_   = !!options.count("replay");
       log_      = !!options.count("log");
       throw_during_startup_ = !!options.count("throw_during_startup");
+      quit_during_startup_ = !!options.count("quit_during_startup");
       dbsize_   = options.at("dbsize").as<uint64_t>();
       log("initialize pluginA");
    }
@@ -44,6 +46,8 @@ class pluginA : public appbase::plugin<pluginA>
       log("starting pluginA");
       if (throw_during_startup_)
          do_throw("throwing as requested");
+      if (quit_during_startup_)
+         appbase::app().quit();
    }
    
    void plugin_shutdown() {
@@ -67,6 +71,7 @@ class pluginA : public appbase::plugin<pluginA>
    bool      readonly_ {false};
    bool      replay_ {false};
    bool      throw_during_startup_ {false};
+   bool      quit_during_startup_ {false};
    bool      log_ {false};
    uint64_t  dbsize_ {0};
    uint32_t* shutdown_counter { nullptr };
@@ -368,6 +373,37 @@ BOOST_AUTO_TEST_CASE(exception_in_startup)
    app_thread.join();
 }
 
+// -----------------------------------------------------------------------------
+// Here we make sure that if a plugin calls app().quit() during `plugin_startup()`
+// plugin_shutdown is called for that plugin
+// -----------------------------------------------------------------------------
+BOOST_AUTO_TEST_CASE(quit_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", "--quit_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

From 15db98e4950d14281e21d0c802b2743c81aa4307 Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Tue, 30 May 2023 16:31:36 -0400
Subject: [PATCH 2/2] Small cleanup in tests.

---
 tests/basic_test.cpp | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tests/basic_test.cpp b/tests/basic_test.cpp
index 6debee7..75e54e4 100644
--- a/tests/basic_test.cpp
+++ b/tests/basic_test.cpp
@@ -397,6 +397,7 @@ BOOST_AUTO_TEST_CASE(quit_in_startup)
       try {
          app->startup();
       } catch(const std::exception& e ) {
+         // appbase framework will throw an exception if app.quit() is called during startup
          std::cout << "exception during startup (as expected): " << e.what() << "\n";
       }
       BOOST_CHECK(shutdown_counter == 1); // check that plugin_shutdown() was executed for pA
@@ -424,11 +425,7 @@ BOOST_AUTO_TEST_CASE(queue_emptied_at_quit)
    std::thread app_thread( [&]() {
       app->startup();
       plugin_promise.set_value( {app->get_plugin<pluginA>(), app->get_plugin<pluginB>()} );
-      try {
-         app->exec();
-      } catch(const std::exception& e ) {
-         std::cout << "exception in exec (as expected): " << e.what() << "\n";
-      }
+      app->exec();
    } );
 
    auto [pA, pB] = plugin_fut.get();