From 99d6835dca4e8d02de6f152deb089aeceaabd1b6 Mon Sep 17 00:00:00 2001 From: Dmitry Kalinkin Date: Sun, 22 Feb 2026 21:54:55 -0500 Subject: [PATCH 01/10] Add reproducer to CI tests --- .github/workflows/test_integration_epic.yml | 28 +++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test_integration_epic.yml b/.github/workflows/test_integration_epic.yml index e28882092..8b9ded597 100644 --- a/.github/workflows/test_integration_epic.yml +++ b/.github/workflows/test_integration_epic.yml @@ -134,7 +134,7 @@ jobs: echo "ldd bin/eicrecon" ldd bin/eicrecon - - name: Generate EICrecon input data + - name: Generate EICrecon input data for craterlake uses: eic/run-cvmfs-osg-eic-shell@main with: platform-release: "eic_xl:nightly" @@ -146,7 +146,7 @@ jobs: --gun.momentumMax "20*GeV" --gun.distribution "uniform" -N 20 \ --outputFile sim_e_1GeV_20GeV_craterlake.edm4hep.root -v WARNING - - name: Run EICrecon + - name: Run EICrecon for craterlake uses: eic/run-cvmfs-osg-eic-shell@main with: platform-release: "eic_xl:nightly" @@ -157,3 +157,27 @@ jobs: export LD_LIBRARY_PATH=${GITHUB_WORKSPACE}/../EICrecon/lib:${GITHUB_WORKSPACE}/../EICrecon/lib/EICrecon/plugins:${JANA_HOME}/lib:${JANA_HOME}/lib/JANA/plugins:$LD_LIBRARY_PATH ../EICrecon/bin/eicrecon sim_e_1GeV_20GeV_craterlake.edm4hep.root + - name: Generate EICrecon input data for inner detector + uses: eic/run-cvmfs-osg-eic-shell@main + with: + platform-release: "eic_xl:nightly" + setup: "/opt/detector/epic-main/bin/thisepic.sh" + run: | + echo "--- Generating EICrecon input data ---" + npsim --compactFile ${DETECTOR_PATH}/${DETECTOR}_inner_detector.xml \ + -G --random.seed 1 --gun.particle "e-" --gun.momentumMin "1*GeV" \ + --gun.momentumMax "20*GeV" --gun.distribution "uniform" -N 20 \ + --outputFile sim_e_1GeV_20GeV_inner_detector.edm4hep.root -v WARNING + + - name: Run EICrecon for inner detector + uses: eic/run-cvmfs-osg-eic-shell@main + with: + platform-release: "eic_xl:nightly" + setup: "/opt/detector/epic-main/bin/thisepic.sh" + run: | + export JANA_HOME=$GITHUB_WORKSPACE + export JANA_PLUGIN_PATH=$GITHUB_WORKSPACE/../EICrecon/lib/EICrecon/plugins + export LD_LIBRARY_PATH=${GITHUB_WORKSPACE}/../EICrecon/lib:${GITHUB_WORKSPACE}/../EICrecon/lib/EICrecon/plugins:${JANA_HOME}/lib:${JANA_HOME}/lib/JANA/plugins:$LD_LIBRARY_PATH + export DETECTOR_CONFIG=epic_inner_detector # not important + ../EICrecon/bin/eicrecon sim_e_1GeV_20GeV_inner_detector.edm4hep.root + From 0e536b94e78b62e546808903a19f7a33c9a18ce9 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Mon, 23 Mar 2026 16:28:09 -0400 Subject: [PATCH 02/10] Bugfix: Exceptions in JFactory::Init produced null collections Previously, JFactory::Create() would let DoInit() throw without calling JPodioOutput::LagrangianStore(). This leaves the podio::Frame in a bad state where collections are null instead of empty. --- src/libraries/JANA/JFactory.cc | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index d3d800dd4..b6c396d98 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -42,7 +42,25 @@ void JFactory::Create(const JEvent& event) { } if (mStatus == Status::Uninitialized) { - DoInit(); + try { + DoInit(); + } + catch (...) { + std::cout << "DoInit() failed, catching and storing empty collections" << std::endl; + // If Create() fails due to an exception in Init(), we still save down empty (rather than null) databundles. + // Note that this will not happen if Init() is called eagerly instead of lazily, e.g. by JFactorySet. + // We leave status=Uninitialized, so that future calls to this factory trigger the same exception in Init() + // instead of trying Process() + mStatus = Status::Uninitialized; // This factory has to stay uninitialized + for (auto* output : GetOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + } + for (auto* output : GetVariadicOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + } + std::cout << "Done with DoInit try/catch" << std::endl; + throw; + } } // How do we obtain our data? The priority is as follows: From d04853e5d1a3fd3938c945456f2adb36930abe19 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Tue, 24 Mar 2026 11:37:45 -0400 Subject: [PATCH 03/10] Bugfix: JEvent::GetCollectionBase didn't handle excepted case --- src/libraries/JANA/JEvent.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index 4ce518fcd..e329d170d 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -515,7 +515,7 @@ inline const podio::CollectionBase* JEvent::GetCollectionBase(std::string unique return nullptr; } - if (typed_bundle->GetStatus() == JDatabundle::Status::Empty) { + if (typed_bundle->GetStatus() == JDatabundle::Status::Empty || typed_bundle->GetStatus() == JDatabundle::Status::Excepted) { auto* fac = typed_bundle->GetFactory(); if (fac != nullptr) { JCallGraphEntryMaker cg_entry(mCallGraph, fac); // times execution until this goes out of scope From 454777fea512d021c4ba745cb85b9e793235ce6d Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Tue, 24 Mar 2026 14:00:24 -0400 Subject: [PATCH 04/10] Add test cases for JFactory PodioOutput behavior during exceptions --- .../unit_tests/Components/PodioTests.cc | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index 5c10f71ba..9ab888acc 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -1,4 +1,6 @@ +#include "JANA/JApplicationFwd.h" +#include "PodioDatamodel/ExampleHit.h" #include #include @@ -354,3 +356,117 @@ TEST_CASE("PodioMultifactoryClearData_Test") { } // namespace +namespace podio_exception_tests { + +class ExceptingPodioFactory : public JFactory { + + PodioOutput hits_out {this}; + Parameter except {this, "except", 0, "0: None, 1: in Init(), 2: start of Process(), 3: end of Process()"}; + +public: + ExceptingPodioFactory() { + SetPrefix("myfac"); + } + void Init() override { + LOG_INFO(GetLogger()) << "except=" << *except; + if (*except == 1) { + throw JException("Exception in Init()"); + } + } + void Process(const JEvent& event) override { + if (*except == 2) { + throw JException("Exception in Process() before some data is written"); + } + MutableExampleHit hit; + hit.x(event.GetEventNumber()); + hits_out->push_back(hit); + if (*except == 3) { + throw JException("Exception in Process() after some data is written"); + } + } +}; + + +TEST_CASE("PodioTests_ExceptionInFactoryInit") { + SECTION("ExceptionAtStartOfProcess") { + JApplication app; + app.Add(new JFactoryGeneratorT); + app.SetParameterValue("myfac:except", 2); + app.Initialize(); + + JEvent event(&app); + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + auto frame = event.Get().at(0); + REQUIRE(frame->get("ExampleHit").size() == 0); + + event.Clear(true); + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + frame = event.Get().at(0); + REQUIRE(frame->get("ExampleHit").size() == 0); + } + + SECTION("ExceptionAtEndOfProcess") { + JApplication app; + app.Add(new JFactoryGeneratorT); + app.SetParameterValue("myfac:except", 3); + app.Initialize(); + + JEvent event(&app); + event.SetEventNumber(22); + + std::cout << "Calling failing factory" << std::endl; + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + std::cout << "Calling failing factory" << std::endl; + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + std::cout << "Retrieving collection from frame" << std::endl; + auto frame = event.Get().at(0); + REQUIRE(frame->get("ExampleHit").size() == 1); + + std::cout << "Clearing event" << std::endl; + event.Clear(true); + event.SetEventNumber(23); + + std::cout << "Calling failing factory" << std::endl; + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + std::cout << "Retrieving collection from frame" << std::endl; + + frame = event.Get().at(0); + for (const auto& hit : frame->get("ExampleHit")) { + std::cout << "Hit x=" << hit.x() << std::endl; + } + REQUIRE(frame->get("ExampleHit").size() == 1); + } + +} + +} // namespace + + From acc161ad9fc43b233fc58438c4327d81ae77c869 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Tue, 24 Mar 2026 14:48:58 -0400 Subject: [PATCH 05/10] Refactoring: Merge JFactory::Status, CreationStatus This fixes a longstanding source of confusion in the codebase, and paves the way for storing callback exceptions --- src/libraries/JANA/Components/JHasOutputs.cc | 2 - src/libraries/JANA/JEvent.h | 2 - src/libraries/JANA/JFactory.cc | 3 -- src/libraries/JANA/JFactory.h | 8 +-- src/libraries/JANA/JFactoryT.h | 4 -- src/libraries/JANA/Utils/JInspector.cc | 51 ++++++++++--------- .../janaview/JEventProcessor_janaview.cc | 3 +- .../unit_tests/Components/JFactoryTests.cc | 2 +- 8 files changed, 31 insertions(+), 44 deletions(-) diff --git a/src/libraries/JANA/Components/JHasOutputs.cc b/src/libraries/JANA/Components/JHasOutputs.cc index e430f0c45..35ebb559b 100644 --- a/src/libraries/JANA/Components/JHasOutputs.cc +++ b/src/libraries/JANA/Components/JHasOutputs.cc @@ -6,10 +6,8 @@ void jana::components::UpdateFactoryStatusOnEulerianStore(JFactory* fac) { // that the factory doesn't accidentally get re-run. // We do this inside a weird little free function because we need to avoid creating // a circular definition of JFactory in our templates. - // Eventually we will need to refactor JFactory::Status and CreationStatus. fac->SetStatus(JFactory::Status::Inserted); - fac->SetCreationStatus(JFactory::CreationStatus::Inserted); } diff --git a/src/libraries/JANA/JEvent.h b/src/libraries/JANA/JEvent.h index e329d170d..75e1c367c 100644 --- a/src/libraries/JANA/JEvent.h +++ b/src/libraries/JANA/JEvent.h @@ -449,7 +449,6 @@ inline JFactoryT* JEvent::Insert(T* item, const std::string& tag) const { auto* factory = databundle->GetFactory(); if (factory != nullptr) { factory->SetStatus(JFactory::Status::Inserted); // for when items is empty - factory->SetCreationStatus(JFactory::CreationStatus::Inserted); // for when items is empty factory->SetInsertOrigin( mCallGraph.GetInsertDataOrigin() ); // (see note at top of JCallGraphRecorder.h) return dynamic_cast*>(factory); } @@ -476,7 +475,6 @@ inline JFactoryT* JEvent::Insert(const std::vector& items, const std::str auto* factory = databundle->GetFactory(); if (factory != nullptr) { factory->SetStatus(JFactory::Status::Inserted); // for when items is empty - factory->SetCreationStatus(JFactory::CreationStatus::Inserted); // for when items is empty factory->SetInsertOrigin( mCallGraph.GetInsertDataOrigin() ); // (see note at top of JCallGraphRecorder.h) return dynamic_cast*>(factory); } diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index b6c396d98..291301568 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -106,7 +106,6 @@ void JFactory::Create(const JEvent& event) { if (found_data) { mStatus = Status::Inserted; - mCreationStatus = CreationStatus::InsertedViaGetObjects; return; } } @@ -161,7 +160,6 @@ void JFactory::Create(const JEvent& event) { LOG << "Exception in JFactory::Create, prefix=" << GetPrefix(); mStatus = Status::Excepted; - mCreationStatus = CreationStatus::Created; for (auto* output : GetOutputs()) { output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); } @@ -171,7 +169,6 @@ void JFactory::Create(const JEvent& event) { throw; } mStatus = Status::Processed; - mCreationStatus = CreationStatus::Created; for (auto* output : GetOutputs()) { output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Created); } diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index 2eb03aa21..226ac1154 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -35,7 +35,6 @@ class JFactory : public jana::components::JComponent, public: enum class Status {Uninitialized, Unprocessed, Processed, Inserted, Excepted, Finished}; - enum class CreationStatus { NotCreatedYet, Created, Inserted, InsertedViaGetObjects, NeverCreated }; enum JFactory_Flags_t { JFACTORY_NULL = 0x00, // Not used anywhere @@ -70,14 +69,12 @@ class JFactory : public jana::components::JComponent, std::string GetFactoryName() const { return m_type_name; } Status GetStatus() const { return mStatus; } - CreationStatus GetCreationStatus() const { return mCreationStatus; } JCallGraphRecorder::JDataOrigin GetInsertOrigin() const { return m_insert_origin; } ///< If objects were placed here by JEvent::Insert() this records whether that call was made from a source or factory. uint32_t GetPreviousRunNumber(void) const { return mPreviousRunNumber; } void SetFactoryName(std::string factoryName) { SetTypeName(factoryName); } void SetStatus(Status status){ mStatus = status; } - void SetCreationStatus(CreationStatus status){ mCreationStatus = status; } void SetInsertOrigin(JCallGraphRecorder::JDataOrigin origin) { m_insert_origin = origin; } ///< Called automatically by JEvent::Insert() to records whether that call was made by a source or factory. void SetPreviousRunNumber(uint32_t aRunNumber) { mPreviousRunNumber = aRunNumber; } @@ -99,7 +96,7 @@ class JFactory : public jana::components::JComponent, /// Get data source value depending on how objects came to be here. (Used mainly by JEvent::Get() ) inline JCallGraphRecorder::JDataSource GetDataSource() const { JCallGraphRecorder::JDataSource datasource = JCallGraphRecorder::DATA_FROM_FACTORY; - if( mCreationStatus == JFactory::CreationStatus::Inserted ){ + if( mStatus == JFactory::Status::Inserted ){ if( m_insert_origin == JCallGraphRecorder::ORIGIN_FROM_SOURCE ){ datasource = JCallGraphRecorder::DATA_FROM_SOURCE; }else{ @@ -133,7 +130,6 @@ class JFactory : public jana::components::JComponent, } this->mStatus = JFactory::Status::Unprocessed; - this->mCreationStatus = JFactory::CreationStatus::NotCreatedYet; for (auto* output: GetOutputs()) { output->ClearData(); @@ -189,8 +185,6 @@ class JFactory : public jana::components::JComponent, mutable Status mStatus = Status::Uninitialized; mutable JCallGraphRecorder::JDataOrigin m_insert_origin = JCallGraphRecorder::ORIGIN_NOT_AVAILABLE; // (see note at top of JCallGraphRecorder.h) - - CreationStatus mCreationStatus = CreationStatus::NotCreatedYet; }; // We are moving away from JFactory::GetAs because it only considers the first databundle diff --git a/src/libraries/JANA/JFactoryT.h b/src/libraries/JANA/JFactoryT.h index 64b34cea4..24bb04a74 100644 --- a/src/libraries/JANA/JFactoryT.h +++ b/src/libraries/JANA/JFactoryT.h @@ -88,14 +88,12 @@ class JFactoryT : public JFactory { // from JANA1 and I haven't found a cleaner solution that gives them what they want yet. mOutput.GetDatabundle().SetStatus(JDatabundle::Status::Inserted); mStatus = Status::Inserted; - mCreationStatus = CreationStatus::Inserted; } else { ClearData(); mData = aData; mOutput.GetDatabundle().SetStatus(JDatabundle::Status::Inserted); mStatus = Status::Inserted; - mCreationStatus = CreationStatus::Inserted; } } @@ -104,14 +102,12 @@ class JFactoryT : public JFactory { mData = std::move(aData); mOutput.GetDatabundle().SetStatus(JDatabundle::Status::Inserted); mStatus = Status::Inserted; - mCreationStatus = CreationStatus::Inserted; } virtual void Insert(T* aDatum) { mData.push_back(aDatum); mOutput.GetDatabundle().SetStatus(JDatabundle::Status::Inserted); mStatus = Status::Inserted; - mCreationStatus = CreationStatus::Inserted; } /// Set a flag (or flags) diff --git a/src/libraries/JANA/Utils/JInspector.cc b/src/libraries/JANA/Utils/JInspector.cc index 940d01655..da2925c23 100644 --- a/src/libraries/JANA/Utils/JInspector.cc +++ b/src/libraries/JANA/Utils/JInspector.cc @@ -177,22 +177,23 @@ void JInspector::ToText(const JFactory* fac, bool asJson, std::ostream& out) { auto tag = fac->GetTag(); if (tag.empty()) tag = "(no tag)"; - std::string creationStatus; - switch (fac->GetCreationStatus()) { - case JFactory::CreationStatus::NotCreatedYet: creationStatus = "NotCreatedYet"; break; - case JFactory::CreationStatus::Created: creationStatus = "Created"; break; - case JFactory::CreationStatus::Inserted: creationStatus = "Inserted"; break; - case JFactory::CreationStatus::InsertedViaGetObjects: creationStatus = "InsertedViaGetObjects"; break; - case JFactory::CreationStatus::NeverCreated: creationStatus = "NeverCreated"; break; - default: creationStatus = "Unknown"; + std::string status; + switch (fac->GetStatus()) { + case JFactory::Status::Uninitialized: status = "Uninitialized"; break; + case JFactory::Status::Unprocessed: status = "Unprocessed"; break; + case JFactory::Status::Processed: status = "Processed"; break; + case JFactory::Status::Inserted: status = "Inserted"; break; + case JFactory::Status::Excepted: status = "Excepted"; break; + case JFactory::Status::Finished: status = "Finished"; break; + default: status = "Unknown"; } if (!asJson) { out << "Plugin name " << pluginName << std::endl; - // out << "Factory name " << factoryName << std::endl; + out << "Type name " << factoryName << std::endl; out << "Object name " << fac->GetObjectName() << std::endl; out << "Tag " << tag << std::endl; - out << "Creation status " << creationStatus << std::endl; + out << "Status " << status << std::endl; out << "Object count " << fac->GetNumObjects() << std::endl; } else { @@ -201,7 +202,7 @@ void JInspector::ToText(const JFactory* fac, bool asJson, std::ostream& out) { out << " \"factory_name\": \"" << factoryName << "\"," << std::endl; out << " \"object_name\": \"" << fac->GetObjectName() << "\"," << std::endl; out << " \"tag\": \"" << tag << "\"," << std::endl; - out << " \"creation\": \"" << creationStatus << "\"," << std::endl; + out << " \"status\": \"" << status << "\"," << std::endl; out << " \"object_count\": " << fac->GetNumObjects() << "," << std::endl; out << "}" << std::endl; } @@ -220,27 +221,29 @@ void JInspector::ToText(const std::vector& factories, const std::set< for (auto fac : factories) { auto tag = fac->GetTag(); if (tag.empty()) tag = "(no tag)"; - std::string creationStatus; - switch (fac->GetCreationStatus()) { - case JFactory::CreationStatus::NotCreatedYet: creationStatus = "NotCreatedYet"; break; - case JFactory::CreationStatus::Created: creationStatus = "Created"; break; - case JFactory::CreationStatus::Inserted: creationStatus = "Inserted"; break; - case JFactory::CreationStatus::InsertedViaGetObjects: creationStatus = "InsertedViaGetObjects"; break; - case JFactory::CreationStatus::NeverCreated: creationStatus = "NeverCreated"; break; - default: creationStatus = "Unknown"; + std::string status; + switch (fac->GetStatus()) { + case JFactory::Status::Uninitialized: status = "Uninitialized"; break; + case JFactory::Status::Unprocessed: status = "Unprocessed"; break; + case JFactory::Status::Processed: status = "Processed"; break; + case JFactory::Status::Inserted: status = "Inserted"; break; + case JFactory::Status::Excepted: status = "Excepted"; break; + case JFactory::Status::Finished: status = "Finished"; break; + default: status = "Unknown"; } idx += 1; - if (filterlevel > 0 && (fac->GetCreationStatus()==JFactory::CreationStatus::NeverCreated || - fac->GetCreationStatus()==JFactory::CreationStatus::NotCreatedYet)) continue; + if (filterlevel > 0 && (fac->GetStatus() == JFactory::Status::Uninitialized || + fac->GetStatus() == JFactory::Status::Unprocessed)) continue; + if (filterlevel > 1 && (fac->GetNumObjects()== 0)) continue; - if (filterlevel > 2 && (fac->GetCreationStatus()==JFactory::CreationStatus::Inserted || - fac->GetCreationStatus()==JFactory::CreationStatus::InsertedViaGetObjects)) continue; + + if (filterlevel > 2 && (fac->GetStatus() == JFactory::Status::Inserted)) continue; char discrepancy = ' '; auto key = MakeFactoryKey(fac->GetObjectName(), fac->GetTag()); if (discrepancies.find(key) != discrepancies.end()) discrepancy = 'x'; - t | idx | fac->GetObjectName() | tag | creationStatus | fac->GetNumObjects() | discrepancy; + t | idx | fac->GetObjectName() | tag | status | fac->GetNumObjects() | discrepancy; } t.Render(out); } diff --git a/src/plugins/janaview/JEventProcessor_janaview.cc b/src/plugins/janaview/JEventProcessor_janaview.cc index 312a2e9dd..336413a4b 100644 --- a/src/plugins/janaview/JEventProcessor_janaview.cc +++ b/src/plugins/janaview/JEventProcessor_janaview.cc @@ -223,7 +223,8 @@ void JEventProcessor_janaview::GetAssociatedTo(JObject *jobj, vectorGetCreationStatus() == JFactory::CreationStatus::NotCreatedYet) continue; + if(factories[i]->GetStatus() == JFactory::Status::Uninitialized || + (factories[i]->GetStatus() == JFactory::Status::Unprocessed)) continue; // Get objects for this factory and associated objects for each of those vector vobjs = factories[i]->GetAs(); diff --git a/src/programs/unit_tests/Components/JFactoryTests.cc b/src/programs/unit_tests/Components/JFactoryTests.cc index d55a3ccaf..5d6f2e427 100644 --- a/src/programs/unit_tests/Components/JFactoryTests.cc +++ b/src/programs/unit_tests/Components/JFactoryTests.cc @@ -168,7 +168,7 @@ TEST_CASE("JFactoryTests") { SECTION("Issue 135: Users modifying mData directly and calling Set() afterwards") { // Unfortunately, we are giving users the ability to access mData directly. // If they do this, they might think they should call Set() directly afterwards so that - // the Status and CreationStatus flags get correctly set. + // the Status flag gets correctly set. // Previously, this would clear mData before setting it, which would make their data // disappear, and they'd have to dig deep into the JANA internals to find out why. // Now, we account for this case. I still think we should clean up this abstraction, though. From 495f226bd5a0249647d113b543ea574d1756e53b Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Tue, 24 Mar 2026 16:18:59 -0400 Subject: [PATCH 06/10] Factor InitStatus out from JFactory::Status This separates the concern of "Has Init() run?" (which is sticky across ClearData()) from "Where did the data, if any, come from?", which is not --- src/libraries/JANA/JFactory.cc | 13 ++++++------- src/libraries/JANA/JFactory.h | 11 +++++++---- src/libraries/JANA/Utils/JInspector.cc | 11 +++-------- src/plugins/janaview/JEventProcessor_janaview.cc | 3 +-- src/programs/unit_tests/Components/JFactoryTests.cc | 6 +++--- src/programs/unit_tests/Components/PodioTests.cc | 3 ++- 6 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index 291301568..b8879fd7b 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -41,7 +41,7 @@ void JFactory::Create(const JEvent& event) { m_logger = m_app->GetJParameterManager()->GetLogger(GetLoggerName()); } - if (mStatus == Status::Uninitialized) { + if (mInitStatus == InitStatus::NotRun) { try { DoInit(); } @@ -51,7 +51,7 @@ void JFactory::Create(const JEvent& event) { // Note that this will not happen if Init() is called eagerly instead of lazily, e.g. by JFactorySet. // We leave status=Uninitialized, so that future calls to this factory trigger the same exception in Init() // instead of trying Process() - mStatus = Status::Uninitialized; // This factory has to stay uninitialized + mStatus = Status::Excepted; // This factory has to stay uninitialized for (auto* output : GetOutputs()) { output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); } @@ -120,7 +120,7 @@ void JFactory::Create(const JEvent& event) { // Otherwise we call Process() to create the data in the first place. // If we already ran Process() but it excepted, we re-run Process() to trigger the same exception, so that every consumer // is forced to handle it. Otherwise one "fault-tolerant" consumer will swallow the exception for everybody else. - if (mStatus == Status::Unprocessed || mStatus == Status::Excepted) { + if (mStatus == Status::Empty || mStatus == Status::Excepted) { auto run_number = event.GetRunNumber(); if (mPreviousRunNumber != run_number) { if (m_callback_style == CallbackStyle::LegacyMode) { @@ -179,7 +179,7 @@ void JFactory::Create(const JEvent& event) { } void JFactory::DoInit() { - if (mStatus != Status::Uninitialized) { + if (mInitStatus != InitStatus::NotRun) { return; } for (auto* parameter : m_parameters) { @@ -189,16 +189,15 @@ void JFactory::DoInit() { service->Fetch(m_app); } CallWithJExceptionWrapper("JFactory::Init", [&](){ Init(); }); - mStatus = Status::Unprocessed; + mInitStatus = InitStatus::Run; } void JFactory::DoFinish() { - if (mStatus == Status::Unprocessed || mStatus == Status::Processed) { + if (mInitStatus == InitStatus::Run) { if (mPreviousRunNumber != -1) { CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); } CallWithJExceptionWrapper("JFactory::Finish", [&](){ Finish(); }); - mStatus = Status::Finished; } } diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index 226ac1154..d6406aa68 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -34,7 +34,8 @@ class JFactory : public jana::components::JComponent, public jana::components::JHasOutputs { public: - enum class Status {Uninitialized, Unprocessed, Processed, Inserted, Excepted, Finished}; + enum class InitStatus {NotRun, Run, Excepted}; + enum class Status {Empty, Processed, Inserted, Excepted}; enum JFactory_Flags_t { JFACTORY_NULL = 0x00, // Not used anywhere @@ -115,7 +116,8 @@ class JFactory : public jana::components::JComponent, } void ClearData() { - if (this->mStatus == JFactory::Status::Uninitialized) { + + if (this->mStatus == JFactory::Status::Empty) { return; } @@ -129,7 +131,7 @@ class JFactory : public jana::components::JComponent, return; } - this->mStatus = JFactory::Status::Unprocessed; + this->mStatus = JFactory::Status::Empty; for (auto* output: GetOutputs()) { output->ClearData(); @@ -183,7 +185,8 @@ class JFactory : public jana::components::JComponent, bool mInsideCreate = false; // Use this to detect cycles in factory dependencies std::string mObjectName; - mutable Status mStatus = Status::Uninitialized; + mutable Status mStatus = Status::Empty; + mutable InitStatus mInitStatus = InitStatus::NotRun; mutable JCallGraphRecorder::JDataOrigin m_insert_origin = JCallGraphRecorder::ORIGIN_NOT_AVAILABLE; // (see note at top of JCallGraphRecorder.h) }; diff --git a/src/libraries/JANA/Utils/JInspector.cc b/src/libraries/JANA/Utils/JInspector.cc index da2925c23..6a593de86 100644 --- a/src/libraries/JANA/Utils/JInspector.cc +++ b/src/libraries/JANA/Utils/JInspector.cc @@ -179,12 +179,10 @@ void JInspector::ToText(const JFactory* fac, bool asJson, std::ostream& out) { std::string status; switch (fac->GetStatus()) { - case JFactory::Status::Uninitialized: status = "Uninitialized"; break; - case JFactory::Status::Unprocessed: status = "Unprocessed"; break; + case JFactory::Status::Empty: status = "Empty"; break; case JFactory::Status::Processed: status = "Processed"; break; case JFactory::Status::Inserted: status = "Inserted"; break; case JFactory::Status::Excepted: status = "Excepted"; break; - case JFactory::Status::Finished: status = "Finished"; break; default: status = "Unknown"; } @@ -223,17 +221,14 @@ void JInspector::ToText(const std::vector& factories, const std::set< if (tag.empty()) tag = "(no tag)"; std::string status; switch (fac->GetStatus()) { - case JFactory::Status::Uninitialized: status = "Uninitialized"; break; - case JFactory::Status::Unprocessed: status = "Unprocessed"; break; + case JFactory::Status::Empty: status = "Empty"; break; case JFactory::Status::Processed: status = "Processed"; break; case JFactory::Status::Inserted: status = "Inserted"; break; case JFactory::Status::Excepted: status = "Excepted"; break; - case JFactory::Status::Finished: status = "Finished"; break; default: status = "Unknown"; } idx += 1; - if (filterlevel > 0 && (fac->GetStatus() == JFactory::Status::Uninitialized || - fac->GetStatus() == JFactory::Status::Unprocessed)) continue; + if (filterlevel > 0 && (fac->GetStatus() == JFactory::Status::Empty)) continue; if (filterlevel > 1 && (fac->GetNumObjects()== 0)) continue; diff --git a/src/plugins/janaview/JEventProcessor_janaview.cc b/src/plugins/janaview/JEventProcessor_janaview.cc index 336413a4b..9452e8424 100644 --- a/src/plugins/janaview/JEventProcessor_janaview.cc +++ b/src/plugins/janaview/JEventProcessor_janaview.cc @@ -223,8 +223,7 @@ void JEventProcessor_janaview::GetAssociatedTo(JObject *jobj, vectorGetStatus() == JFactory::Status::Uninitialized || - (factories[i]->GetStatus() == JFactory::Status::Unprocessed)) continue; + if(factories[i]->GetStatus() == JFactory::Status::Empty) continue; // Get objects for this factory and associated objects for each of those vector vobjs = factories[i]->GetAs(); diff --git a/src/programs/unit_tests/Components/JFactoryTests.cc b/src/programs/unit_tests/Components/JFactoryTests.cc index 5d6f2e427..e489cb938 100644 --- a/src/programs/unit_tests/Components/JFactoryTests.cc +++ b/src/programs/unit_tests/Components/JFactoryTests.cc @@ -220,7 +220,7 @@ TEST_CASE("JFactoryTests") { LOG << "JFactoryTests: Exception in JFactory::Process" << LOG_END; auto event = std::make_shared(); JFactoryTestExceptingFactory fac; - REQUIRE(fac.GetStatus() == JFactory::Status::Uninitialized); + REQUIRE(fac.GetStatus() == JFactory::Status::Empty); REQUIRE_THROWS(fac.CreateAndGetData(event)); REQUIRE(fac.GetStatus() == JFactory::Status::Excepted); @@ -231,10 +231,10 @@ TEST_CASE("JFactoryTests") { LOG << "JFactoryTests: Exception in JFactory::Init" << LOG_END; auto event = std::make_shared(); JFactoryTestExceptingInInitFactory fac; - REQUIRE(fac.GetStatus() == JFactory::Status::Uninitialized); + REQUIRE(fac.GetStatus() == JFactory::Status::Empty); REQUIRE_THROWS(fac.CreateAndGetData(event)); - REQUIRE(fac.GetStatus() == JFactory::Status::Uninitialized); + REQUIRE(fac.GetStatus() == JFactory::Status::Excepted); REQUIRE_THROWS(fac.CreateAndGetData(event)); } } diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index 9ab888acc..a0b1b79cd 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -355,7 +355,7 @@ TEST_CASE("PodioMultifactoryClearData_Test") { } // TEST_CASE } // namespace - +/* namespace podio_exception_tests { class ExceptingPodioFactory : public JFactory { @@ -468,5 +468,6 @@ TEST_CASE("PodioTests_ExceptionInFactoryInit") { } } // namespace +*/ From 21dbf6e9e447891ae46fa563a2b541c4712de604 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Tue, 24 Mar 2026 23:30:11 -0400 Subject: [PATCH 07/10] JFactory::Create stores and re-throws exceptions Previously, it would re-run the excepting code instead. --- src/libraries/JANA/JFactory.cc | 103 +++++++++++------- src/libraries/JANA/JFactory.h | 7 +- .../unit_tests/Components/PodioTests.cc | 33 +++++- 3 files changed, 96 insertions(+), 47 deletions(-) diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index b8879fd7b..8e9ed5a4c 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -41,28 +41,6 @@ void JFactory::Create(const JEvent& event) { m_logger = m_app->GetJParameterManager()->GetLogger(GetLoggerName()); } - if (mInitStatus == InitStatus::NotRun) { - try { - DoInit(); - } - catch (...) { - std::cout << "DoInit() failed, catching and storing empty collections" << std::endl; - // If Create() fails due to an exception in Init(), we still save down empty (rather than null) databundles. - // Note that this will not happen if Init() is called eagerly instead of lazily, e.g. by JFactorySet. - // We leave status=Uninitialized, so that future calls to this factory trigger the same exception in Init() - // instead of trying Process() - mStatus = Status::Excepted; // This factory has to stay uninitialized - for (auto* output : GetOutputs()) { - output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); - } - for (auto* output : GetVariadicOutputs()) { - output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); - } - std::cout << "Done with DoInit try/catch" << std::endl; - throw; - } - } - // How do we obtain our data? The priority is as follows: // 1. JFactory::Process() if REGENERATE flag is set // 2. JEvent::Insert() @@ -116,26 +94,57 @@ void JFactory::Create(const JEvent& event) { // 4. JFactory::Process() // --------------------------------------------------------------------- - // If the data was Processed (instead of Inserted), it will be in cache, and we can just exit. - // Otherwise we call Process() to create the data in the first place. - // If we already ran Process() but it excepted, we re-run Process() to trigger the same exception, so that every consumer - // is forced to handle it. Otherwise one "fault-tolerant" consumer will swallow the exception for everybody else. - if (mStatus == Status::Empty || mStatus == Status::Excepted) { - auto run_number = event.GetRunNumber(); - if (mPreviousRunNumber != run_number) { - if (m_callback_style == CallbackStyle::LegacyMode) { - if (mPreviousRunNumber != -1) { - CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); - } - CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event.shared_from_this()); }); - CallWithJExceptionWrapper("JFactory::BeginRun", [&](){ BeginRun(event.shared_from_this()); }); - } - else if (m_callback_style == CallbackStyle::ExpertMode) { - CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event); }); - } - mPreviousRunNumber = run_number; + // Check if init had _previously_ excepted but the cache was since cleared + if (mInitStatus == InitStatus::Excepted && mStatus == Status::Empty) { + for (auto* output : GetOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); } + for (auto* output : GetVariadicOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + } + mStatus = Status::Excepted; + std::rethrow_exception(mException); + } + + // Make sure Init() ran, which might except... + try { + DoInit(); // This checks mInitStatus internally before calling Init() + } + catch(...) { + // If Init() excepts, we still need to store an empty collection + mStatus = Status::Excepted; + for (auto* output : GetOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + } + for (auto* output : GetVariadicOutputs()) { + output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); + } + std::rethrow_exception(mException); + } + + // At this point, Init() has run and has _not_ excepted + + if (mStatus == Status::Excepted) { + // But Process() might have already excepted! + std::rethrow_exception(mException); + } + else if (mStatus == Status::Empty) { + // Now we know that we need to run Process() to create the data in the first place try { + auto run_number = event.GetRunNumber(); + if (mPreviousRunNumber != run_number) { + if (m_callback_style == CallbackStyle::LegacyMode) { + if (mPreviousRunNumber != -1) { + CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); + } + CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event.shared_from_this()); }); + CallWithJExceptionWrapper("JFactory::BeginRun", [&](){ BeginRun(event.shared_from_this()); }); + } + else if (m_callback_style == CallbackStyle::ExpertMode) { + CallWithJExceptionWrapper("JFactory::ChangeRun", [&](){ ChangeRun(event); }); + } + mPreviousRunNumber = run_number; + } for (auto* input : GetInputs()) { input->Populate(event); } @@ -160,6 +169,7 @@ void JFactory::Create(const JEvent& event) { LOG << "Exception in JFactory::Create, prefix=" << GetPrefix(); mStatus = Status::Excepted; + mException = std::current_exception(); for (auto* output : GetOutputs()) { output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); } @@ -168,6 +178,8 @@ void JFactory::Create(const JEvent& event) { } throw; } + + // Save the (successfully processed) data mStatus = Status::Processed; for (auto* output : GetOutputs()) { output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Created); @@ -188,8 +200,15 @@ void JFactory::DoInit() { for (auto* service : m_services) { service->Fetch(m_app); } - CallWithJExceptionWrapper("JFactory::Init", [&](){ Init(); }); - mInitStatus = InitStatus::Run; + try { + CallWithJExceptionWrapper("JFactory::Init", [&](){ Init(); }); + mInitStatus = InitStatus::Run; + } + catch (...) { + mInitStatus = InitStatus::Excepted; + mException = std::current_exception(); + throw; + } } void JFactory::DoFinish() { diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index d6406aa68..c0a4fd0ce 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -185,9 +185,10 @@ class JFactory : public jana::components::JComponent, bool mInsideCreate = false; // Use this to detect cycles in factory dependencies std::string mObjectName; - mutable Status mStatus = Status::Empty; - mutable InitStatus mInitStatus = InitStatus::NotRun; - mutable JCallGraphRecorder::JDataOrigin m_insert_origin = JCallGraphRecorder::ORIGIN_NOT_AVAILABLE; // (see note at top of JCallGraphRecorder.h) + Status mStatus = Status::Empty; + InitStatus mInitStatus = InitStatus::NotRun; + JCallGraphRecorder::JDataOrigin m_insert_origin = JCallGraphRecorder::ORIGIN_NOT_AVAILABLE; // (see note at top of JCallGraphRecorder.h) + std::exception_ptr mException; }; // We are moving away from JFactory::GetAs because it only considers the first databundle diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index a0b1b79cd..f0a33ae74 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -355,7 +355,6 @@ TEST_CASE("PodioMultifactoryClearData_Test") { } // TEST_CASE } // namespace -/* namespace podio_exception_tests { class ExceptingPodioFactory : public JFactory { @@ -388,6 +387,37 @@ class ExceptingPodioFactory : public JFactory { TEST_CASE("PodioTests_ExceptionInFactoryInit") { + SECTION("ExceptionInInit") { + JApplication app; + app.Add(new JFactoryGeneratorT); + app.SetParameterValue("myfac:except", 1); + app.Initialize(); + + JEvent event(&app); + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + auto frame = event.Get().at(0); + REQUIRE(frame->get("ExampleHit").size() == 0); + + event.Clear(true); + + try { + event.GetCollectionBase("ExampleHit"); + REQUIRE(0 == 1); + } catch (...) { } + + frame = event.Get().at(0); + REQUIRE(frame->get("ExampleHit").size() == 0); + } SECTION("ExceptionAtStartOfProcess") { JApplication app; app.Add(new JFactoryGeneratorT); @@ -468,6 +498,5 @@ TEST_CASE("PodioTests_ExceptionInFactoryInit") { } } // namespace -*/ From 396f168f5ce00320b8dbd2ff1f7600f8cbf09ba8 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Wed, 25 Mar 2026 12:03:48 -0400 Subject: [PATCH 08/10] Document new JFactory state machine --- docs/behavior.md | 112 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/docs/behavior.md b/docs/behavior.md index 553db1f76..585f4fac1 100644 --- a/docs/behavior.md +++ b/docs/behavior.md @@ -3,6 +3,118 @@ ## Introduction +## Components + +### JFactories + +#### Callbacks + +The user-defined callbacks are `Init`, `Process`, `BeginRun`, `EndRun`, and `Finish`. + +- `Init` is run at most once, and is used for loading and caching constant data. It is guaranteed to run before any call to `Process`. + If `Process` is never called, e.g. because the data is `Insert`ed instead, then `Init` will not be called. + +- `Finish` only runs if `Init` has ran, and is responsible for cleaning up and closing any state opened by `Init`. + +- `BeginRun` runs after `Init` and before `Process`. It will only be called if the run number has been set, and this will be the first call to `Process` corresponding to that run number. + `BeginRun` is responsible for loading and caching data keyed off of the run number, e.g. conditions, calibrations, lookup tables, machine learning models. + +- `EndRun` runs after a previously set run number gets changed. It runs after the last call to `Process` with that run number, and before the `BeginRun` for the new run number. + `EndRun` is responsible for cleaning up and closing any state opened by the previous call to `BeginRun`. `EndRun` is guaranteed to be called exactly once for each `BeginRun`, as long + as JANA2 is shut down cleanly. + +- `Process` is called exactly once for every JEvent. By the time `Process` is called, JANA2 guarantees that `Init` will have been called, followed by `BeginRun` if the run number has been set. + +#### Activation +JFactories are lazy by design, which means that they won't be activated unless requested by another component. Activation is defined as calling `JFactory::Create` with a given JEvent, which +triggers the running of zero or more factory callbacks. Users are discouraged from calling `JFactory::Create` directly; instead, factories are usually activated via any of the following mechanisms: + +- Declaring an `Input` helper member variable on another JComponent. +- Calling `JEvent::Get*()` or `JEvent::GetCollection()` +- Setting the parameter `jana:autoactivate=$DATABUNDLE_NAME`. This will activate the factory even though its results are never used downstream, which is mainly useful for debugging. + +#### Exception handling +JFactory's user-defined methods are allowed to throw exceptions. Unlike other JANA2 components, throwing an exception here does not immediately terminate processing -- the +user has the opportunity to catch the exception in the caller. For instance, calls to `JEvent::Get*` may be wrapped in a try-catch block. The exception itself is wrapped in a `JException` which preserves +stack trace and component information. If a factory callback excepts, the exception is stored so that it can be re-thrown on future calls. The excepting callback will only be called once. If any +callbacks except, JANA2 will still store the contents of each `Output` helper's transient output buffer. This means that if `Process` excepts, all data inserted prior to the exception will be preserved. + +#### State machine + +The JFactory state machine is defined as follows. The state has two components, `InitStatus` and `Status`. `InitStatus <- {NotRun, Run, Excepted}`, which allows `JFactory::Create` to guarantee that +`Init` gets called at most once, even when some events have the factory data `Insert`ed and others let it be `Processed`. `Status <- {Empty, Processed, Inserted, Excepted}` captures what exactly +is in cache. `Empty` is the only state where no data has been cached/stored; `Excepted` means that an empty or partial collection was stored. This storage operation is guaranteed to happen exactly once +per activated factory per event, a requirement imposed by Podio's write-exactly-once semantics. The following transition diagram shows the relationship between valid states and transitions corresponding +to factory callbacks. + +```mermaid + +stateDiagram-v2 + NE : (InitNotRun, Empty) + XE : (InitExcepted, Empty) + XX : (InitExcepted, Excepted) + RE : (InitRun, Empty) + RI : (InitRun, Inserted) + RP : (InitRun, Processed) + RX : (InitRun, Excepted) + NI : (InitNotRun, Inserted) + XI : (InitExcepted, Inserted) + + [*] --> NE + NE --> RE: Init + RE --> RP: Process + RP --> RE: ClearData + RE --> RX: Process + RX --> RE: ClearData + RE --> RI: Insert + RI --> RE: ClearData + NE --> NI: Insert + NI --> NE: ClearData + NE --> XE: Init + XE --> XX: Process + XX --> XE: ClearData + XE --> XI: Insert + XI --> XE: ClearData +``` + +Note that many of these transitions are encapsulated behind `JFactory::Create`. The only operations available to the user are `Create` (i.e. activate), `Insert`, and `ClearData`. +Redrawing the state diagram in terms of these transitions gives us: + +```mermaid +stateDiagram-v2 + NE : (InitNotRun, Empty) + XE : (InitExcepted, Empty) + XX : (InitExcepted, Excepted) + RE : (InitRun, Empty) + RI : (InitRun, Inserted) + RP : (InitRun, Processed) + RX : (InitRun, Excepted) + NI : (InitNotRun, Inserted) + XI : (InitExcepted, Inserted) + + [*] --> NE + NE --> RP: Create + RE --> RP: Create + RP --> RE: ClearData + RE --> RX: Create + RX --> RE: ClearData + RE --> RI: Insert + RI --> RE: ClearData + NE --> NI: Insert + NI --> NE: ClearData + NE --> XX: Create + XE --> XX: Create + XX --> XE: ClearData + XE --> XI: Insert + XI --> XE: ClearData + NE --> RX: Create +``` + +Although not shown for the sake of visual clarity, it is important to note that `Create` operations are idempotent, so all of the `Status:Inserted`, `Status:Processed`, and `Status:Excepted` states +have implicit `Create` transitions pointing back to themselves. Correspondingly, the `Status:Empty` states have `ClearData` transitions pointing back to themselves. However, the `Status:Inserted` states +do _not_ have `Insert` transitions pointing back to themselves. Multiple `Insert` operations are disallowed due to the write-once constraint. + + ## Execution Engine ### Engine initialization From 97bb3071a067f1c3afa0c0940e898af2140a2ae5 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sun, 29 Mar 2026 13:37:24 -0400 Subject: [PATCH 09/10] Test interleaved Insert and Process --- src/libraries/JANA/JFactory.cc | 10 +++---- src/libraries/JANA/JFactory.h | 5 ++-- .../unit_tests/Components/PodioTests.cc | 30 +++++++++++++++++++ 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/libraries/JANA/JFactory.cc b/src/libraries/JANA/JFactory.cc index 8e9ed5a4c..685e23006 100644 --- a/src/libraries/JANA/JFactory.cc +++ b/src/libraries/JANA/JFactory.cc @@ -95,7 +95,7 @@ void JFactory::Create(const JEvent& event) { // --------------------------------------------------------------------- // Check if init had _previously_ excepted but the cache was since cleared - if (mInitStatus == InitStatus::Excepted && mStatus == Status::Empty) { + if (mInitStatus == InitStatus::InitExcepted && mStatus == Status::Empty) { for (auto* output : GetOutputs()) { output->LagrangianStore(*event.GetFactorySet(), JDatabundle::Status::Excepted); } @@ -191,7 +191,7 @@ void JFactory::Create(const JEvent& event) { } void JFactory::DoInit() { - if (mInitStatus != InitStatus::NotRun) { + if (mInitStatus != InitStatus::InitNotRun) { return; } for (auto* parameter : m_parameters) { @@ -202,17 +202,17 @@ void JFactory::DoInit() { } try { CallWithJExceptionWrapper("JFactory::Init", [&](){ Init(); }); - mInitStatus = InitStatus::Run; + mInitStatus = InitStatus::InitRun; } catch (...) { - mInitStatus = InitStatus::Excepted; + mInitStatus = InitStatus::InitExcepted; mException = std::current_exception(); throw; } } void JFactory::DoFinish() { - if (mInitStatus == InitStatus::Run) { + if (mInitStatus == InitStatus::InitRun) { if (mPreviousRunNumber != -1) { CallWithJExceptionWrapper("JFactory::EndRun", [&](){ EndRun(); }); } diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index c0a4fd0ce..3f90117ca 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -34,7 +34,7 @@ class JFactory : public jana::components::JComponent, public jana::components::JHasOutputs { public: - enum class InitStatus {NotRun, Run, Excepted}; + enum class InitStatus {InitNotRun, InitRun, InitExcepted}; enum class Status {Empty, Processed, Inserted, Excepted}; enum JFactory_Flags_t { @@ -70,6 +70,7 @@ class JFactory : public jana::components::JComponent, std::string GetFactoryName() const { return m_type_name; } Status GetStatus() const { return mStatus; } + InitStatus GetInitStatus() const { return mInitStatus; } JCallGraphRecorder::JDataOrigin GetInsertOrigin() const { return m_insert_origin; } ///< If objects were placed here by JEvent::Insert() this records whether that call was made from a source or factory. uint32_t GetPreviousRunNumber(void) const { return mPreviousRunNumber; } @@ -186,7 +187,7 @@ class JFactory : public jana::components::JComponent, std::string mObjectName; Status mStatus = Status::Empty; - InitStatus mInitStatus = InitStatus::NotRun; + InitStatus mInitStatus = InitStatus::InitNotRun; JCallGraphRecorder::JDataOrigin m_insert_origin = JCallGraphRecorder::ORIGIN_NOT_AVAILABLE; // (see note at top of JCallGraphRecorder.h) std::exception_ptr mException; }; diff --git a/src/programs/unit_tests/Components/PodioTests.cc b/src/programs/unit_tests/Components/PodioTests.cc index f0a33ae74..91a2439e9 100644 --- a/src/programs/unit_tests/Components/PodioTests.cc +++ b/src/programs/unit_tests/Components/PodioTests.cc @@ -1,4 +1,5 @@ +#include "JANA/Components/JDatabundle.h" #include "JANA/JApplicationFwd.h" #include "PodioDatamodel/ExampleHit.h" #include @@ -494,7 +495,36 @@ TEST_CASE("PodioTests_ExceptionInFactoryInit") { } REQUIRE(frame->get("ExampleHit").size() == 1); } +} + +TEST_CASE("PodioTests_InterleavedInsertAndProcess") { + JApplication app; + app.Add(new JFactoryGeneratorT); + app.SetParameterValue("myfac:except", 0); // Disable exceptions + app.Initialize(); + + JEvent event(&app); + + auto databundle = event.GetFactorySet()->GetDatabundle("ExampleHit"); + REQUIRE(databundle->GetStatus() == JDatabundle::Status::Empty); + REQUIRE(databundle->GetFactory() != nullptr); + REQUIRE(databundle->GetFactory()->GetStatus() == JFactory::Status::Empty); + REQUIRE(databundle->GetFactory()->GetInitStatus() == JFactory::InitStatus::InitNotRun); + + + // Insert hits. This will NOT trigger Init + ExampleHitCollection hits; + hits.push_back(MutableExampleHit()); + event.InsertCollection(std::move(hits), "ExampleHit"); + + event.GetCollectionBase("ExampleHit"); + REQUIRE(databundle->GetFactory()->GetInitStatus() == JFactory::InitStatus::InitNotRun); + + event.Clear(); + // Trigger Init and Process + event.GetCollectionBase("ExampleHit"); + REQUIRE(databundle->GetFactory()->GetInitStatus() == JFactory::InitStatus::InitRun); } } // namespace From 521dac34251fce4cd31d4c1db64a5bda3464a768 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Sun, 29 Mar 2026 14:21:51 -0400 Subject: [PATCH 10/10] Deprecated JFactory::GetCreationStatus() for GlueX --- src/libraries/JANA/JFactory.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libraries/JANA/JFactory.h b/src/libraries/JANA/JFactory.h index 3f90117ca..0dc2b6eea 100644 --- a/src/libraries/JANA/JFactory.h +++ b/src/libraries/JANA/JFactory.h @@ -35,6 +35,7 @@ class JFactory : public jana::components::JComponent, public: enum class InitStatus {InitNotRun, InitRun, InitExcepted}; + enum class CreationStatus { NotCreatedYet, Created }; enum class Status {Empty, Processed, Inserted, Excepted}; enum JFactory_Flags_t { @@ -71,6 +72,11 @@ class JFactory : public jana::components::JComponent, std::string GetFactoryName() const { return m_type_name; } Status GetStatus() const { return mStatus; } InitStatus GetInitStatus() const { return mInitStatus; } + [[deprecated]] + CreationStatus GetCreationStatus() const { + if (mStatus == Status::Empty) return CreationStatus::NotCreatedYet; + return CreationStatus::Created; + } JCallGraphRecorder::JDataOrigin GetInsertOrigin() const { return m_insert_origin; } ///< If objects were placed here by JEvent::Insert() this records whether that call was made from a source or factory. uint32_t GetPreviousRunNumber(void) const { return mPreviousRunNumber; }