Bugfixes: JServices

.github/workflows/ccpp-linux.yml

@@ -51,4 +51,4 @@ jobs:
     - name: jana-unit-tests
       run: |
         echo "--- Running jana-unit-tests ------------------------------"
-        ctest --test-dir build -R jana-unit-tests
+        ctest --test-dir build -R jana-unit-tests --rerun-failed --output-on-failure

src/libraries/JANA/Components/JOmniFactory.h

@@ -251,7 +251,7 @@ class JOmniFactory : public JMultifactory, public jana::components::JHasInputs {
                variadic_output_count += 1;
             }
         }
-        size_t variadic_output_collection_count = FindVariadicCollectionCount(m_outputs.size(), variadic_output_count, output_collection_names.size(), true);
+        size_t variadic_output_collection_count = FindVariadicCollectionCount(m_outputs.size(), variadic_output_count, output_collection_names.size(), false);
 
         // Set output collection names and create corresponding helper factories
         i = 0;

src/libraries/JANA/JApplication.cc

@@ -152,6 +152,10 @@ void JApplication::Initialize() {
 
     topology_builder->create_topology();
     auto execution_engine = m_service_locator->get<JExecutionEngine>();
+
+    // Make sure that Init() is called on any remaining JServices
+    m_service_locator->wire_everything();
+
     m_initialized = true;
     // This needs to be at the end so that m_initialized==false while InitPlugin() is being called
 }

src/libraries/JANA/Services/JServiceLocator.h

@@ -46,6 +46,7 @@ class JServiceLocator {
         std::lock_guard<std::mutex> lock(mutex);
         auto svc = std::dynamic_pointer_cast<JService>(t);
         assert(svc != nullptr);
+        svc->SetTypeName(JTypeInfo::demangle<T>());
         underlying[std::type_index(typeid(T))] = svc;
     }
 

src/libraries/JANA/Services/JWiringService.h

@@ -29,7 +29,7 @@ class JWiringService : public JService {
     };
 
 private:
-    Parameter<std::string> m_wirings_input_file {this, "jana:wiring_file", "wiring.toml", 
+    Parameter<std::string> m_wirings_input_file {this, "jana:wiring_file", "", 
         "Path to TOML file containing wiring definitions"};
 
     Parameter<bool> m_strict_inheritance {this, "jana:wiring_strictness", true,

src/programs/unit_tests/CMakeLists.txt

@@ -22,6 +22,7 @@ set(TEST_SOURCES
     Components/JFactoryTests.cc
     Components/JFactoryGeneratorTests.cc
     Components/JMultiFactoryTests.cc
+    Components/JServiceTests.cc
     Components/UnfoldTests.cc
     Components/UserExceptionTests.cc
 

src/programs/unit_tests/Components/JFactoryTests.cc

@@ -4,6 +4,7 @@
 
 
 #include "JANA/Components/JComponentFwd.h"
+#include "JANA/JApplicationFwd.h"
 #include "JANA/JFactory.h"
 #include "catch.hpp"
 #include "JFactoryTests.h"
@@ -566,4 +567,43 @@ TEST_CASE("JFactory_ExceptionHandling") {
     }
 }
 
-
+TEST_CASE("JFactory_GetObjects_Caching") {
+    JApplication app;
+    app.Add(new JFactoryGeneratorT<JFactoryT<JFactoryTestDummyObject>>());
+    app.Add(new JFactoryGeneratorT<JFactoryT<DifferentDummyObject>>());
+    auto source = new JFactoryTestDummySource;
+    auto event = std::make_shared<JEvent>(&app);
+    event->SetJEventSource(source);
+
+    SECTION("RepeatedGetObjectsAreCached") {
+        auto dummies = event->Get<JFactoryTestDummyObject>();
+        REQUIRE(dummies.at(0)->data == 8);
+        REQUIRE(source->get_objects_count == 1);
+        REQUIRE(source->get_objects_dummy_count == 1);
+
+        dummies = event->Get<JFactoryTestDummyObject>();
+        REQUIRE(dummies.at(0)->data == 8);
+        REQUIRE(source->get_objects_count == 1);
+        REQUIRE(source->get_objects_dummy_count == 1);
+    }
+
+    SECTION("DifferentGetObjectsAreNotCached") {
+        auto dummies = event->Get<JFactoryTestDummyObject>();
+        REQUIRE(dummies.at(0)->data == 8);
+        REQUIRE(source->get_objects_count == 1);
+        REQUIRE(source->get_objects_dummy_count == 1);
+        REQUIRE(source->get_objects_different_count == 0);
+
+        auto different = event->Get<DifferentDummyObject>();
+        REQUIRE(different.at(0)->E == 123.0);
+        REQUIRE(source->get_objects_count == 2);
+        REQUIRE(source->get_objects_dummy_count == 1);
+        REQUIRE(source->get_objects_different_count == 1);
+
+        different = event->Get<DifferentDummyObject>();
+        REQUIRE(different.at(0)->E == 123.0);
+        REQUIRE(source->get_objects_count == 2);
+        REQUIRE(source->get_objects_dummy_count == 1);
+        REQUIRE(source->get_objects_different_count == 1);
+    }
+}

src/programs/unit_tests/Components/JFactoryTests.h

@@ -23,6 +23,11 @@ struct JFactoryTestDummyObject : public JObject {
     }
 };
 
+struct DifferentDummyObject : public JObject {
+    double E;
+    DifferentDummyObject(double E) : E(E) {}
+};
+
 
 
 /// DummyFactory is a trivial JFactory which reports how often its member functions get called
@@ -69,6 +74,10 @@ struct JFactoryTestExceptingInInitFactory : public JFactoryT<JFactoryTestDummyOb
 
 struct JFactoryTestDummySource: public JEventSource {
 
+    int get_objects_count = 0;
+    int get_objects_dummy_count = 0;
+    int get_objects_different_count = 0;
+
     JFactoryTestDummySource() {
         SetCallbackStyle(CallbackStyle::ExpertMode);
         EnableGetObjects();
@@ -79,10 +88,19 @@ struct JFactoryTestDummySource: public JEventSource {
     };
 
     bool GetObjects(const std::shared_ptr<const JEvent>&, JFactory* aFactory) override {
-        auto factory = dynamic_cast<JFactoryT<JFactoryTestDummyObject>*>(aFactory);
-        if (factory != nullptr) {
-            factory->Insert(new JFactoryTestDummyObject(8));
-            factory->Insert(new JFactoryTestDummyObject(88));
+        get_objects_count += 1;
+        auto dummy_factory = dynamic_cast<JFactoryT<JFactoryTestDummyObject>*>(aFactory);
+        if (dummy_factory != nullptr) {
+            get_objects_dummy_count += 1;
+            dummy_factory->Insert(new JFactoryTestDummyObject(8));
+            dummy_factory->Insert(new JFactoryTestDummyObject(88));
+            return true;
+        }
+        auto different_factory = dynamic_cast<JFactoryT<DifferentDummyObject>*>(aFactory);
+        if (different_factory != nullptr) {
+            get_objects_different_count += 1;
+            different_factory->Insert(new DifferentDummyObject(123.));
+            return true;
         }
         return false;
     }

src/programs/unit_tests/Components/JServiceTests.cc

@@ -0,0 +1,82 @@
+
+#include <catch.hpp>
+#include <JANA/JApplication.h>
+#include <JANA/JService.h>
+#include <JANA/Components/JOmniFactory.h>
+#include <JANA/Components/JOmniFactoryGeneratorT.h>
+
+namespace jana::jservicetests {
+
+class DummyService: public JService {
+    bool init_started = false;
+    bool init_finished = false;
+public:
+    void Init() override {
+        LOG_INFO(GetLogger()) << "Starting DummyService::Init()" << LOG_END;
+        init_started = true;
+        throw std::runtime_error("Something goes wrong");
+        init_finished = true;
+        LOG_INFO(GetLogger()) << "Finishing DummyService::Init()" << LOG_END;
+    }
+};
+
+TEST_CASE("JServiceTests_ExceptionInInit") {
+    JApplication app;
+    app.ProvideService(std::make_shared<DummyService>());
+    try {
+        app.Initialize();
+        REQUIRE(1 == 0); // Shouldn't be reachable
+
+        auto sut = app.GetService<DummyService>();
+        REQUIRE(1 == 0); // Definitely shouldn't be reachable
+    }
+    catch (JException& e) {
+        REQUIRE(e.GetMessage() == "Something goes wrong");
+        REQUIRE(e.type_name == "jana::jservicetests::DummyService");
+        REQUIRE(e.function_name == "JService::Init");
+    }
+}
+
+struct DummyData {int x;};
+
+struct DummyOmniFactory: public jana::components::JOmniFactory<DummyOmniFactory> {
+
+    Service<DummyService> m_svc {this};
+    Output<DummyData> m_output {this};
+
+    void Configure() {
+    }
+
+    void ChangeRun(int32_t /*run_nr*/) {
+    }
+
+    void Execute(int32_t /*run_nr*/, uint64_t /*evt_nr*/) {
+        m_output().push_back(new DummyData{22});
+    }
+};
+
+TEST_CASE("JServiceTests_ExceptionInInit_Issue381") {
+    JApplication app;
+    app.ProvideService(std::make_shared<DummyService>());
+
+    auto gen = new components::JOmniFactoryGeneratorT<DummyOmniFactory>();
+    gen->AddWiring("dummy", {}, {"data"});
+    app.Add(gen);
+
+    try {
+        app.Initialize();
+        REQUIRE(1 == 0); // Shouldn't be reachable
+        auto event = std::make_shared<JEvent>(&app);
+        auto data = event->Get<DummyData>("data");
+        REQUIRE(1 == 0); // Definitely shouldn't be reachable
+        REQUIRE(data.at(0)->x == 22);
+    }
+    catch (JException& e) {
+        REQUIRE(e.GetMessage() == "Something goes wrong");
+        REQUIRE(e.type_name == "jana::jservicetests::DummyService");
+        REQUIRE(e.function_name == "JService::Init");
+    }
+}
+
+} // namespace jana::jservicetests
+