fix: Dangling reference due to source link lifetime in fitters [backp…

…ort #2095 to develop/v25.x] (#2098)

Backport 3918f38 from #2095.
---
Closes #2000

the source links in the GFS went out of scope and caused segfaults

- this PR harmonizes the source link handling for the different fitters
- fixes a potential UB caused by parameter evaluation order
- fixes a potential dangling reference in `SourceLinkAdapterIterator`

Co-authored-by: Andreas Stefl <487211+andiwand@users.noreply.github.com>

Core/include/Acts/EventData/SourceLink.hpp

@@ -117,10 +117,7 @@ struct SourceLinkAdapterIterator {
     return !(*this == other);
   }
 
-  Acts::SourceLink operator*() const {
-    const auto& sl = *m_iterator;
-    return Acts::SourceLink{sl};
-  }
+  Acts::SourceLink operator*() const { return Acts::SourceLink{*m_iterator}; }
 
   auto operator-(const SourceLinkAdapterIterator& other) const {
     return m_iterator - other.m_iterator;

Core/include/Acts/TrackFitting/Chi2Fitter.hpp

@@ -241,9 +241,7 @@ class Chi2Fitter {
     using result_type = Chi2FitterResult<traj_t>;
 
     /// Allows retrieving measurements for a surface
-    const std::map<GeometryIdentifier,
-                   std::reference_wrapper<const SourceLink>>*
-        inputMeasurements = nullptr;
+    const std::map<GeometryIdentifier, SourceLink>* inputMeasurements = nullptr;
 
     /// Whether to consider multiple scattering.
     bool multipleScattering = false;  // TODO: add later
@@ -651,12 +649,11 @@ class Chi2Fitter {
     // We need to copy input SourceLinks anyways, so the map can own them.
     ACTS_VERBOSE("preparing " << std::distance(it, end)
                               << " input measurements");
-    std::map<GeometryIdentifier, std::reference_wrapper<const SourceLink>>
-        inputMeasurements;
-
+    std::map<GeometryIdentifier, SourceLink> inputMeasurements;
     for (; it != end; ++it) {
-      const SourceLink& sl = *it;
-      inputMeasurements.emplace(sl.geometryId(), sl);
+      SourceLink sl = *it;
+      auto geoId = sl.geometryId();
+      inputMeasurements.emplace(geoId, std::move(sl));
     }
 
     // TODO: for now, we use STL objects to collect the information during

Core/include/Acts/TrackFitting/GaussianSumFitter.hpp

@@ -234,11 +234,11 @@ struct GaussianSumFitter {
     // We need to copy input SourceLinks anyways, so the map can own them.
     ACTS_VERBOSE("Preparing " << std::distance(begin, end)
                               << " input measurements");
-    std::map<GeometryIdentifier, std::reference_wrapper<const SourceLink>>
-        inputMeasurements;
+    std::map<GeometryIdentifier, SourceLink> inputMeasurements;
     for (auto it = begin; it != end; ++it) {
-      const SourceLink& sl = *it;
-      inputMeasurements.emplace(sl.geometryId(), sl);
+      SourceLink sl = *it;
+      auto geoId = sl.geometryId();
+      inputMeasurements.emplace(geoId, std::move(sl));
     }
 
     ACTS_VERBOSE(
@@ -259,7 +259,7 @@ struct GaussianSumFitter {
       // Catch the actor and set the measurements
       auto& actor = fwdPropOptions.actionList.template get<GsfActor>();
       actor.setOptions(options);
-      actor.m_cfg.inputMeasurements = inputMeasurements;
+      actor.m_cfg.inputMeasurements = &inputMeasurements;
       actor.m_cfg.numberMeasurements = inputMeasurements.size();
       actor.m_cfg.inReversePass = false;
       actor.m_cfg.logger = m_actorLogger.get();
@@ -326,7 +326,7 @@ struct GaussianSumFitter {
 
       auto& actor = bwdPropOptions.actionList.template get<GsfActor>();
       actor.setOptions(options);
-      actor.m_cfg.inputMeasurements = inputMeasurements;
+      actor.m_cfg.inputMeasurements = &inputMeasurements;
       actor.m_cfg.inReversePass = true;
       actor.m_cfg.logger = m_actorLogger.get();
       actor.setOptions(options);

Core/include/Acts/TrackFitting/KalmanFitter.hpp

@@ -1051,7 +1051,8 @@ class KalmanFitter {
     // for (const auto& sl : sourcelinks) {
     for (; it != end; ++it) {
       SourceLink sl = *it;
-      inputMeasurements.emplace(sl.geometryId(), std::move(sl));
+      auto geoId = sl.geometryId();
+      inputMeasurements.emplace(geoId, std::move(sl));
     }
 
     // Create the ActionList and AbortList

Core/include/Acts/TrackFitting/detail/GsfActor.hpp

@@ -75,8 +75,7 @@ struct GsfActor {
     std::size_t maxComponents = 16;
 
     /// Input measurements
-    std::map<GeometryIdentifier, std::reference_wrapper<const SourceLink>>
-        inputMeasurements;
+    const std::map<GeometryIdentifier, SourceLink>* inputMeasurements = nullptr;
 
     /// Bethe Heitler Approximator pointer. The fitter holds the approximator
     /// instance TODO if we somehow could initialize a reference here...
@@ -219,12 +218,12 @@ struct GsfActor {
 
     // Check what we have on this surface
     const auto found_source_link =
-        m_cfg.inputMeasurements.find(surface.geometryId());
+        m_cfg.inputMeasurements->find(surface.geometryId());
     const bool haveMaterial =
         navigator.currentSurface(state.navigation)->surfaceMaterial() &&
         !m_cfg.disableAllMaterialHandling;
     const bool haveMeasurement =
-        found_source_link != m_cfg.inputMeasurements.end();
+        found_source_link != m_cfg.inputMeasurements->end();
 
     ACTS_VERBOSE(std::boolalpha << "haveMaterial " << haveMaterial
                                 << ", haveMeasurement: " << haveMeasurement);