Bugfix: Components not being destroyed when they should

We are using shared_ptrs to manage the lifetimes of all JServices. JServiceLocator was holding onto some of these shared_ptrs and wasn't being deleted. The fix for now is to make sure JServiceLocator gets deleted when JApplication does. The longer-term fix is to stop holding on to JServices using shared_ptrs, since users may trivially create cycles. An even better option is to use Service<T> helpers with weak or raw pointers internally.

This addresses issue #284.

src/libraries/JANA/JApplication.cc

@@ -39,7 +39,7 @@ JApplication::JApplication(JParameterManager* params) {
     }
     m_component_manager = std::make_shared<JComponentManager>();
     m_plugin_loader = std::make_shared<JPluginLoader>();
-    m_service_locator = new JServiceLocator;
+    m_service_locator = std::make_unique<JServiceLocator>();
 
     ProvideService(m_params);
     ProvideService(m_component_manager);

src/libraries/JANA/JApplicationFwd.h

@@ -135,7 +135,8 @@ class JApplication {
 private:
 
     JLogger m_logger;
-    JServiceLocator* m_service_locator;
+
+    std::unique_ptr<JServiceLocator> m_service_locator;
 
     std::shared_ptr<JParameterManager> m_params;
     std::shared_ptr<JPluginLoader> m_plugin_loader;

src/programs/unit_tests/Components/JEventProcessorTests.cc

@@ -8,11 +8,15 @@ struct MyEventProcessor : public JEventProcessor {
     int init_count = 0;
     int process_count = 0;
     int finish_count = 0;
+    int* destroy_count = nullptr;
 
     MyEventProcessor() {
         SetCallbackStyle(CallbackStyle::ExpertMode);
         SetTypeName(NAME_OF_THIS);
     }
+    ~MyEventProcessor() {
+        (*destroy_count)++;
+    }
     void Init() override {
         LOG_INFO(GetLogger()) << "Init() called" << LOG_END;
         init_count++;
@@ -32,20 +36,27 @@ TEST_CASE("JEventProcessor_ExpertMode_ProcessCount") {
     LOG << "Running test: JEventProcessor_ExpertMode_ProcessCount" << LOG_END;
 
     auto sut = new MyEventProcessor;
-
-    JApplication app;
-    app.SetParameterValue("log:global", "off");
-    app.SetParameterValue("log:info", "MyEventProcessor");
-    app.SetParameterValue("jana:nevents", 5);
-
-    app.Add(new JEventSource);
-    app.Add(sut);
-    app.Run();
-
-    REQUIRE(sut->init_count == 1);
-    REQUIRE(sut->process_count == 5);
-    REQUIRE(sut->GetEventCount() == 5);
-    REQUIRE(sut->finish_count == 1);
-
+    int destroy_count = 0;
+    sut->destroy_count = &destroy_count;
+
+    {
+        JApplication app;
+        app.SetParameterValue("log:global", "off");
+        app.SetParameterValue("log:info", "MyEventProcessor");
+        app.SetParameterValue("jana:nevents", 5);
+
+        app.Add(new JEventSource);
+        app.Add(sut);
+        app.Run();
+
+        REQUIRE(sut->init_count == 1);
+        REQUIRE(sut->process_count == 5);
+        REQUIRE(sut->GetEventCount() == 5);
+        REQUIRE(sut->finish_count == 1);
+        REQUIRE(destroy_count == 0);
+
+        // App goes out of scope; sut should be destroyed
+    }
+    REQUIRE(destroy_count == 1);
 }