Merge pull request #287 from JeffersonLab/nbrei_service_initialization

Service initialization fixes

src/examples/SubeventCUDAExample/SubeventCUDAExample.cu

@@ -128,10 +128,8 @@ int main() {
                                                                   &subevents_out);
     auto merge_arrow = new JMergeArrow<MyInput, MyOutput>("merge", &processor, &subevents_out, &events_out);
 
-    auto parms = new JParameterManager;
-    // Some params need to be present BEFORE JApplication is constructed, e.g. log levels are lost
-    // parms->SetParameter("log:debug", "JWorker,JScheduler,JArrowProcessingController,JEventProcessorArrow");
-    JApplication app(parms);
+    JApplication app;
+    app.SetParameterValue("log:info", "JWorker,JScheduler,JArrowProcessingController,JEventProcessorArrow");
     app.SetTimeoutEnabled(false);
     app.SetTicker(false);
 

src/examples/SubeventExample/SubeventExample.cc

@@ -88,10 +88,8 @@ int main() {
     auto subprocess_arrow = new JSubeventArrow<MyInput, MyOutput>("subprocess", &processor, &subevents_in, &subevents_out);
     auto merge_arrow = new JMergeArrow<MyInput, MyOutput>("merge", &processor, &subevents_out, &events_out);
 
-    auto parms = new JParameterManager;
-    // Some params need to be present BEFORE JApplication is constructed, e.g. log levels are lost
-    // parms->SetParameter("log:debug", "JWorker,JScheduler,JArrowProcessingController,JEventProcessorArrow");
-    JApplication app(parms);
+    JApplication app;
+    app.SetParameterValue("log:info", "JWorker,JScheduler,JArrowProcessingController,JEventProcessorArrow");
     app.SetTimeoutEnabled(false);
     app.SetTicker(false);
 

src/examples/UnitTestingExample/SimpleClusterFactoryTests.cc

@@ -10,15 +10,11 @@
 
 TEST_CASE("SimpleClusterFactoryTests") {
 
-    // Turn off all unnecessary loggers and turn on the loggers in the factory being tested.
-    // Note this needs to happen BEFORE creating the JApplication.
-    auto params = new JParameterManager;
-    params->SetParameter("log:debug", "SimpleClusterFactory");
-    params->SetParameter("log:off", "JApplication,JParameterManager,JArrowProcessingController,JArrow");
-
     // We need to fire up the JApplication so that our Factory can access all of its JServices.
     // However, for unit testing, we don't need (or want!) to set up an event source or actually call JApplication::Run().
-    JApplication app(params);
+    JApplication app;
+    app.SetParameterValue("log:debug", "SimpleClusterFactory");
+    app.SetParameterValue("log:off", "JApplication,JParameterManager,JArrowProcessingController,JArrow");
     // Add any plugins you need here
     // app.AddPlugin("myPlugin");
     app.Initialize();  // Load the plugins
@@ -75,4 +71,4 @@ TEST_CASE("SimpleClusterFactoryTests") {
         REQUIRE(true == true);
     }
 
-}
+}

src/libraries/JANA/JApplication.cc

@@ -9,53 +9,42 @@
 #include <JANA/Services/JParameterManager.h>
 #include <JANA/Services/JPluginLoader.h>
 #include <JANA/Services/JComponentManager.h>
+#include <JANA/Services/JLoggingService.h>
 #include <JANA/Services/JGlobalRootLock.h>
 #include <JANA/Engine/JArrowProcessingController.h>
 #include <JANA/Utils/JCpuInfo.h>
 #include <JANA/Engine/JTopologyBuilder.h>
 
 JApplication *japp = nullptr;
 
-JApplication::JApplication(JLogger::Level verbosity) {
-    m_service_locator = new JServiceLocator;
-    m_params = std::make_shared<JParameterManager>();
-    m_params->SetParameter("log:global", verbosity);
-    m_service_locator->provide(m_params);
-    ProvideService(m_params);
-    ProvideService(std::make_shared<JLoggingService>());
-    ProvideService(std::make_shared<JPluginLoader>());
-    ProvideService(std::make_shared<JComponentManager>());
-    ProvideService(std::make_shared<JGlobalRootLock>());
-    ProvideService(std::make_shared<JTopologyBuilder>());
-
-    m_plugin_loader = m_service_locator->get<JPluginLoader>();
-    m_component_manager = m_service_locator->get<JComponentManager>();
-    m_logger = m_service_locator->get<JLoggingService>()->get_logger("JApplication");
-    m_logger.show_classname = false;
-}
 
 JApplication::JApplication(JParameterManager* params) {
 
+    // We set up some very essential services here, but note 
+    // that they won't have been initialized yet. We need them now
+    // so that they can passively receive components, plugin names, parameter values, etc.
+    // These passive operations don't require any parameters, services, or
+    // logging output, so they don't need to be initialized until later.
+    // They will be fully initialized in JApplication::Initialize().
+    // Only then they will be exposed to the user through the service locator.
+
     if (params == nullptr) {
         m_params = std::make_shared<JParameterManager>();
     }
     else {
         m_params = std::shared_ptr<JParameterManager>(params);
     }
-
+    m_component_manager = std::make_shared<JComponentManager>();
+    m_plugin_loader = std::make_shared<JPluginLoader>();
     m_service_locator = new JServiceLocator;
+
     ProvideService(m_params);
+    ProvideService(m_component_manager);
+    ProvideService(m_plugin_loader);
     ProvideService(std::make_shared<JLoggingService>());
-    ProvideService(std::make_shared<JPluginLoader>());
-    ProvideService(std::make_shared<JComponentManager>());
     ProvideService(std::make_shared<JGlobalRootLock>());
     ProvideService(std::make_shared<JTopologyBuilder>());
 
-    m_plugin_loader = m_service_locator->get<JPluginLoader>();
-    m_component_manager = m_service_locator->get<JComponentManager>();
-
-    m_logger = m_service_locator->get<JLoggingService>()->get_logger("JApplication");
-    m_logger.show_classname = false;
 }
 
 
@@ -107,28 +96,36 @@ void JApplication::Add(JEventUnfolder* unfolder) {
 }
 
 
-// Controlling processing
-
 void JApplication::Initialize() {
 
     /// Initialize the application in preparation for data processing.
-    /// This is called by the Run method so users will usually not
-    /// need to call this directly.
+    /// This is called by the Run method so users will usually not need to call this directly.
 
     // Only run this once
     if (m_initialized) return;
 
-    // Obtain final values of parameters and loggers
-    m_plugin_loader->InitPhase2();
-    m_component_manager->InitPhase2();
-    m_logger = m_service_locator->get<JLoggingService>()->get_logger("JApplication");
+    // Now that all parameters, components, plugin names, etc have been set, 
+    // we can expose our builtin services to the user via GetService()
+    m_services_available = true;
+
+    // We trigger initialization 
+    auto logging_service = m_service_locator->get<JLoggingService>();
+    auto component_manager = m_service_locator->get<JComponentManager>();
+    auto plugin_loader = m_service_locator->get<JPluginLoader>();
+    auto topology_builder = m_service_locator->get<JTopologyBuilder>();
+
+    // Set logger on JApplication itself
+    m_logger = logging_service->get_logger("JApplication");
     m_logger.show_classname = false;
 
     // Attach all plugins
-    m_plugin_loader->attach_plugins(m_component_manager.get());
+    plugin_loader->attach_plugins(component_manager.get());
 
     // Resolve all event sources now that all plugins have been loaded
-    m_component_manager->resolve_event_sources();
+    component_manager->resolve_event_sources();
+
+    // Give all components a JApplication pointer and a logger
+    component_manager->configure_components();
 
     // Set desired nthreads. We parse the 'nthreads' parameter two different ways for backwards compatibility.
     m_desired_nthreads = 1;
@@ -150,16 +147,15 @@ void JApplication::Initialize() {
         LOG_WARN(m_logger) << "Unrecognized engine choice! Falling back to jana:engine=0" << LOG_END;
     }
     */
-    std::shared_ptr<JTopologyBuilder> topology_builder = m_service_locator->get<JTopologyBuilder>();
     auto topology = topology_builder->get_or_create();
+    ProvideService(std::make_shared<JArrowProcessingController>(topology));
 
-    auto japc = std::make_shared<JArrowProcessingController>(topology);
-    m_service_locator->provide(japc);  // Make concrete class available via SL
     m_processing_controller = m_service_locator->get<JArrowProcessingController>();  // Get deps from SL
-    m_service_locator->provide(m_processing_controller);  // Make abstract class available via SL
+    ProvideService(m_processing_controller);  // Make abstract class available via SL
     m_processing_controller->initialize();
 
     m_initialized = true;
+    // This needs to be at the end so that m_initialized==false while InitPlugin() is being called
 }
 
 /// @brief Run the application, launching 1 or more threads to do the work.

src/libraries/JANA/JApplication.h

@@ -18,6 +18,9 @@ T JApplication::GetParameterValue(std::string name) {
 /// A convenience method which delegates to JParameterManager
 template<typename T>
 JParameter* JApplication::SetParameterValue(std::string name, T val) {
+    if (m_initialized) {
+        throw JException("SetParameterValue() must be called before Initialize(), as otherwise the parameter value won't be used!");
+    }
     return m_params->SetParameter(name, val);
 }
 
@@ -39,12 +42,22 @@ JParameter* JApplication::GetParameter(std::string name, T& result) {
 /// A convenience method which delegates to JServiceLocator
 template <typename T>
 std::shared_ptr<T> JApplication::GetService() {
+    if (!m_services_available) {
+        LOG_WARN(m_logger) << "GetService() called before Initialize(): Any parameter values set after this point won't be used!" << LOG_END;
+        // Eventually, GetService() could trigger Initialize() just like Run() does. 
+        // In order to make this happen, JTopologyBuilder needs modification.
+        // The blockers are SubeventExample, TopologyTests, SubeventTests
+        //throw JException("Application needs initialization before services become available");
+    }
     return m_service_locator->get<T>();
 }
 
 /// A convenience method which delegates to JServiceLocator
 template <typename T>
 void JApplication::ProvideService(std::shared_ptr<T> service) {
+    if (m_initialized) {
+        throw JException("Services need to be provided before JApplication::Initialize(), or inside InitPlugin()");
+    }
     service->SetApplication(this);
     m_service_locator->provide(service);
 }

src/libraries/JANA/JApplicationFwd.h

@@ -146,6 +146,7 @@ class JApplication {
     bool m_draining_queues = false;
     bool m_skip_join = false;
     std::atomic_bool m_initialized {false};
+    std::atomic_bool m_services_available {false};
     bool m_ticker_on = true;
     bool m_timeout_on = true;
     bool m_extended_report = false;

src/libraries/JANA/Omni/JComponent.h

@@ -91,9 +91,19 @@ class JComponent::Service : public JComponent::ServiceBase {
     }
 
     ServiceT& operator()() {
+        if (m_data == nullptr) {
+            throw JException("Attempted to access a Service which hasn't been attached to this Component yet!");
+        }
         return *m_data;
     }
 
+    ServiceT* operator->() {
+        if (m_data == nullptr) {
+            throw JException("Attempted to access a Service which hasn't been attached to this Component yet!");
+        }
+        return m_data.get();
+    }
+
 protected:
 
     void Init(JApplication* app) {

src/libraries/JANA/Services/JComponentManager.cc

@@ -30,15 +30,13 @@ JComponentManager::~JComponentManager() {
     }
 }
 
-void JComponentManager::InitPhase2() {
+void JComponentManager::Init() {
 
-    // We don't set these in Init() because Init() gets called by the JApplication constructor and we want to give the user a chance to 
-    // set them manually before they call JApplication::Init().
-    m_params().SetDefaultParameter("event_source_type", m_user_evt_src_typename, "Manually specifies which JEventSource should open the input file");
-    m_params().SetDefaultParameter("record_call_stack", m_enable_call_graph_recording, "Records a trace of who called each factory. Reduces performance but necessary for plugins such as janadot.");
-    m_params().SetDefaultParameter("jana:nevents", m_nevents, "Max number of events that sources can emit");
-    m_params().SetDefaultParameter("jana:nskip", m_nskip, "Number of events that sources should skip before starting emitting");
-    m_params().FilterParameters(m_default_tags, "DEFTAG:");
+    m_params->SetDefaultParameter("event_source_type", m_user_evt_src_typename, "Manually specifies which JEventSource should open the input file");
+    m_params->SetDefaultParameter("record_call_stack", m_enable_call_graph_recording, "Records a trace of who called each factory. Reduces performance but necessary for plugins such as janadot.");
+    m_params->SetDefaultParameter("jana:nevents", m_nevents, "Max number of events that sources can emit");
+    m_params->SetDefaultParameter("jana:nskip", m_nskip, "Number of events that sources should skip before starting emitting");
+    m_params->FilterParameters(m_default_tags, "DEFTAG:");
 
     // Look for factories to auto-activate
     // Right now AutoActivator parameter won't show up in parameters list. Reconsider this.
@@ -47,6 +45,29 @@ void JComponentManager::InitPhase2() {
     }
 }
 
+void JComponentManager::configure_components() {
+    for (auto* src : m_evt_srces) {
+        src->SetApplication(GetApplication());
+        src->SetLogger(m_logging->get_logger(src->GetLoggerName()));
+    }
+    for (auto* proc : m_evt_procs) {
+        proc->SetApplication(GetApplication());
+        proc->SetLogger(m_logging->get_logger(proc->GetLoggerName()));
+    }
+    for (auto* fac_gen : m_fac_gens) {
+        fac_gen->SetApplication(GetApplication());
+        //fac_gen->SetLogger(m_logging->get_logger(fac_gen->GetLoggerName()));
+    }
+    for (auto* src_gen : m_src_gens) {
+        src_gen->SetJApplication(GetApplication());
+        //src_gen->SetLogger(m_logging->get_logger(src_gen->GetLoggerName()));
+    }
+    for (auto* unfolder : m_unfolders) {
+        unfolder->SetApplication(GetApplication());
+        unfolder->SetLogger(m_logging->get_logger(unfolder->GetLoggerName()));
+    }
+}
+
 void JComponentManager::next_plugin(std::string plugin_name) {
     // We defer resolving event sources until we have finished loading all plugins
     m_current_plugin_name = plugin_name;
@@ -58,36 +79,26 @@ void JComponentManager::add(std::string event_source_name) {
 
 void JComponentManager::add(JEventSourceGenerator *source_generator) {
     source_generator->SetPluginName(m_current_plugin_name);
-    source_generator->SetJApplication(GetApplication());
-    // source_generator->SetLogger(m_logging().get_logger(source_generator->GetLoggerName()));
     m_src_gens.push_back(source_generator);
 }
 
 void JComponentManager::add(JFactoryGenerator *factory_generator) {
     factory_generator->SetPluginName(m_current_plugin_name);
-    factory_generator->SetApplication(GetApplication());
-    // factory_generator->SetLogger(m_logging().get_logger(factory_generator->GetLoggerName()));
     m_fac_gens.push_back(factory_generator);
 }
 
 void JComponentManager::add(JEventSource *event_source) {
     event_source->SetPluginName(m_current_plugin_name);
-    event_source->SetApplication(GetApplication());
-    event_source->SetLogger(m_logging().get_logger(event_source->GetLoggerName()));
     m_evt_srces.push_back(event_source);
 }
 
 void JComponentManager::add(JEventProcessor *processor) {
     processor->SetPluginName(m_current_plugin_name);
-    processor->SetApplication(GetApplication());
-    processor->SetLogger(m_logging().get_logger(processor->GetLoggerName()));
     m_evt_procs.push_back(processor);
 }
 
 void JComponentManager::add(JEventUnfolder* unfolder) {
     unfolder->SetPluginName(m_current_plugin_name);
-    unfolder->SetApplication(GetApplication());
-    unfolder->SetLogger(m_logging().get_logger(unfolder->GetLoggerName()));
     m_unfolders.push_back(unfolder);
 }
 
@@ -99,6 +110,7 @@ void JComponentManager::configure_event(JEvent& event) {
 }
 
 
+
 void JComponentManager::resolve_event_sources() {
 
     m_user_evt_src_gen = resolve_user_event_source_generator();

src/libraries/JANA/Services/JComponentManager.h

@@ -20,7 +20,7 @@ class JComponentManager : public JService {
 
     explicit JComponentManager();
     ~JComponentManager() override;
-    void InitPhase2();
+    void Init() override;
 
     void next_plugin(std::string plugin_name);
 
@@ -31,6 +31,7 @@ class JComponentManager : public JService {
     void add(JEventProcessor* processor);
     void add(JEventUnfolder* unfolder);
 
+    void configure_components();
     void resolve_event_sources();
     JEventSourceGenerator* resolve_user_event_source_generator() const;
     JEventSourceGenerator* resolve_event_source(std::string source_name) const;

src/libraries/JANA/Services/JLoggingService.cc

@@ -30,8 +30,11 @@ inline void JParameterManager::Parse(const std::string& in, JLogger::Level& out)
     else if (std::strcmp(token.c_str(), "fatal") == 0) { 
         out = JLogger::Level::FATAL;
     }
+    else if (std::strcmp(token.c_str(), "off") == 0) { 
+        out = JLogger::Level::OFF;
+    }
     else {
-        throw JException("Unable to parse log level: '%s'. Options are: TRACE, DEBUG, INFO, WARN, ERROR, FATAL", in.c_str());
+        throw JException("Unable to parse log level: '%s'. Options are: TRACE, DEBUG, INFO, WARN, ERROR, FATAL, OFF", in.c_str());
     }
 }
 

src/libraries/JANA/Services/JPluginLoader.cc

@@ -14,12 +14,12 @@
 
 class JApplication;
 
-void JPluginLoader::InitPhase2() {
+void JPluginLoader::Init() {
 
-    m_params().SetDefaultParameter("plugins", m_plugins_to_include, "Comma-separated list of plugins to load.");
-    m_params().SetDefaultParameter("plugins_to_ignore", m_plugins_to_exclude, "Comma-separated list of plugins to NOT load, even if they are specified in 'plugins'.");
-    m_params().SetDefaultParameter("jana:plugin_path", m_plugin_paths_str, "Colon-separated list of paths to search for plugins");
-    m_params().SetDefaultParameter("jana:debug_plugin_loading", m_verbose, "Trace the plugin search path and display any loading errors");
+    m_params->SetDefaultParameter("plugins", m_plugins_to_include, "Comma-separated list of plugins to load.");
+    m_params->SetDefaultParameter("plugins_to_ignore", m_plugins_to_exclude, "Comma-separated list of plugins to NOT load, even if they are specified in 'plugins'.");
+    m_params->SetDefaultParameter("jana:plugin_path", m_plugin_paths_str, "Colon-separated list of paths to search for plugins");
+    m_params->SetDefaultParameter("jana:debug_plugin_loading", m_verbose, "Trace the plugin search path and display any loading errors");
 
     if (m_verbose) {
         // The jana:debug_plugin_loading parameter is kept around for backwards compatibility