From 11707c371ca1c05647cff6bbb084cdc355b811b3 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila <37295697+m-fila@users.noreply.github.com> Date: Thu, 12 Sep 2024 15:53:08 +0200 Subject: [PATCH] Fix data race in EventHeaderCreator (#224) --- k4FWCore/components/EventHeaderCreator.cpp | 6 +- k4FWCore/components/EventHeaderCreator.h | 4 +- test/k4FWCoreTest/CMakeLists.txt | 3 +- test/k4FWCoreTest/options/CheckOutputFiles.py | 21 +++++++ .../options/createEventHeaderConcurrent.py | 59 +++++++++++++++++++ 5 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 test/k4FWCoreTest/options/createEventHeaderConcurrent.py diff --git a/k4FWCore/components/EventHeaderCreator.cpp b/k4FWCore/components/EventHeaderCreator.cpp index 1326e62a..182e1efc 100644 --- a/k4FWCore/components/EventHeaderCreator.cpp +++ b/k4FWCore/components/EventHeaderCreator.cpp @@ -26,13 +26,13 @@ EventHeaderCreator::EventHeaderCreator(const std::string& name, ISvcLocator* svc "Name of the EventHeaderCollection that will be stored in the output root file."); } -StatusCode EventHeaderCreator::execute(const EventContext&) const { - static int eventNumber = 0; +StatusCode EventHeaderCreator::execute(const EventContext& ctx) const { + auto eventNumber = ctx.evt(); debug() << "Filling EventHeader with runNumber " << int(m_runNumber) << " and eventNumber " << eventNumber + m_eventNumberOffset << endmsg; auto headers = m_headerCol.createAndPut(); auto header = headers->create(); header.setRunNumber(m_runNumber); - header.setEventNumber(eventNumber++ + m_eventNumberOffset); + header.setEventNumber(eventNumber + m_eventNumberOffset); return StatusCode::SUCCESS; } diff --git a/k4FWCore/components/EventHeaderCreator.h b/k4FWCore/components/EventHeaderCreator.h index a3b4dc40..c84ed458 100644 --- a/k4FWCore/components/EventHeaderCreator.h +++ b/k4FWCore/components/EventHeaderCreator.h @@ -19,6 +19,7 @@ #ifndef K4FWCORE_EVENTHEADERCREATOR #define K4FWCORE_EVENTHEADERCREATOR +#include #include "Gaudi/Algorithm.h" #include "k4FWCore/DataHandle.h" @@ -44,7 +45,8 @@ class EventHeaderCreator : public Gaudi::Algorithm { this, "eventNumberOffset", 0, "Event number offset, eventNumber will be filled with 'event_index + eventNumberOffset'"}; // datahandle for the EventHeader - mutable DataHandle m_headerCol{"EventHeader", Gaudi::DataHandle::Writer, this}; + mutable DataHandle m_headerCol{edm4hep::labels::EventHeader, + Gaudi::DataHandle::Writer, this}; }; #endif diff --git a/test/k4FWCoreTest/CMakeLists.txt b/test/k4FWCoreTest/CMakeLists.txt index e8d9e22c..e65fc048 100644 --- a/test/k4FWCoreTest/CMakeLists.txt +++ b/test/k4FWCoreTest/CMakeLists.txt @@ -137,12 +137,13 @@ add_test_with_env(FunctionalCollectionMerger options/ExampleFunctionalCollection add_test_with_env(FunctionalFilterFile options/ExampleFunctionalFilterFile.py) add_test_with_env(FunctionalMetadata options/ExampleFunctionalMetadata.py) add_test_with_env(FunctionalMetadataOldAlgorithm options/ExampleFunctionalMetadataOldAlgorithm.py) +add_test_with_env(createEventHeaderConcurrent options/createEventHeaderConcurrent.py) add_test_with_env(FunctionalWrongImport options/ExampleFunctionalWrongImport.py) set_tests_properties(FunctionalWrongImport PROPERTIES PASS_REGULAR_EXPRESSION "ImportError: Importing ApplicationMgr or IOSvc from Configurables is not allowed.") add_test(NAME FunctionalCheckFiles COMMAND python3 ${CMAKE_CURRENT_LIST_DIR}/options/CheckOutputFiles.py) -set_tests_properties(FunctionalCheckFiles PROPERTIES DEPENDS "FunctionalFile;FunctionalMTFile;FunctionalMultipleFile;FunctionalOutputCommands;FunctionalProducerAbsolutePath;FunctionalTransformerRuntimeEmpty;FunctionalMix;FunctionalMixIOSvc;FunctionalTransformerHist;FunctionalCollectionMerger;FunctionalFilterFile;FunctionalMetadata;FunctionalMetadataOldAlgorithm") +set_tests_properties(FunctionalCheckFiles PROPERTIES DEPENDS "FunctionalFile;FunctionalMTFile;FunctionalMultipleFile;FunctionalOutputCommands;FunctionalProducerAbsolutePath;FunctionalTransformerRuntimeEmpty;FunctionalMix;FunctionalMixIOSvc;FunctionalTransformerHist;FunctionalCollectionMerger;FunctionalFilterFile;FunctionalMetadata;FunctionalMetadataOldAlgorithm;createEventHeaderConcurrent") # Do this after checking the files not to overwrite them add_test_with_env(FunctionalFile_toolong options/ExampleFunctionalFile.py -n 999 PROPERTIES DEPENDS FunctionalCheckFiles PASS_REGULAR_EXPRESSION diff --git a/test/k4FWCoreTest/options/CheckOutputFiles.py b/test/k4FWCoreTest/options/CheckOutputFiles.py index 665c2562..90c8971d 100644 --- a/test/k4FWCoreTest/options/CheckOutputFiles.py +++ b/test/k4FWCoreTest/options/CheckOutputFiles.py @@ -188,3 +188,24 @@ def check_events(filename, number): raise RuntimeError( f"Metadata parameter {key} does not match the expected value, got {metadata.get_parameter(key)} but expected {value}" ) + +reader = podio.root_io.Reader("eventHeaderConcurrent.root") +events = reader.get("events") +expected_events_length = 10 +expected_run_number = 42 +expected_event_numbers = set(range(42, 42 + expected_events_length)) +seen_event_numbers = set() +if len(events) != expected_events_length: + raise RuntimeError("Number of events does not match expected number") +for frame in events: + event_header = frame.get("EventHeader")[0] + if (run_number := event_header.getRunNumber()) != expected_run_number: + raise RuntimeError( + f"Run number is not set correctly (expected {expected_run_number}, actual {run_number})" + ) + event_number = event_header.getEventNumber() + if event_number not in expected_event_numbers: + raise RuntimeError(f"Event number {event_number} is not in expected numbers") + if event_number in seen_event_numbers: + raise RuntimeError(f"Event number {event_number} is duplicated") + seen_event_numbers.add(event_number) diff --git a/test/k4FWCoreTest/options/createEventHeaderConcurrent.py b/test/k4FWCoreTest/options/createEventHeaderConcurrent.py new file mode 100644 index 00000000..10e8f4d0 --- /dev/null +++ b/test/k4FWCoreTest/options/createEventHeaderConcurrent.py @@ -0,0 +1,59 @@ +# +# Copyright (c) 2014-2024 Key4hep-Project. +# +# This file is part of Key4hep. +# See https://key4hep.github.io/key4hep-doc/ for further info. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +# This is an example creating EventHeader created in GaudiHive + +from Gaudi.Configuration import * + +from Configurables import EventHeaderCreator +from Configurables import HiveSlimEventLoopMgr, HiveWhiteBoard, AvalancheSchedulerSvc +from k4FWCore import ApplicationMgr, IOSvc + +evtslots = 6 +threads = 6 + +whiteboard = HiveWhiteBoard( + "EventDataSvc", + EventSlots=evtslots, + ForceLeaves=True, +) + +slimeventloopmgr = HiveSlimEventLoopMgr( + "HiveSlimEventLoopMgr", SchedulerName="AvalancheSchedulerSvc", OutputLevel=WARNING +) + +scheduler = AvalancheSchedulerSvc(ThreadPoolSize=threads, ShowDataFlow=True, OutputLevel=WARNING) + + +eventHeaderCreator = EventHeaderCreator( + "eventHeaderCreator", runNumber=42, eventNumberOffset=42, OutputLevel=DEBUG +) + +svc = IOSvc("IOSvc") +svc.output = "eventHeaderConcurrent.root" + +ApplicationMgr( + TopAlg=[eventHeaderCreator], + EvtSel="NONE", + EvtMax=10, + ExtSvc=[whiteboard], + EventLoop=slimeventloopmgr, + MessageSvcType="InertMessageSvc", + OutputLevel=INFO, +)