[geom] Fix TLS ownership and lifecycle

README/ReleaseNotes/v642/index.md

@@ -36,6 +36,7 @@ The following people have contributed to this new version:
  Devajith Valaparambil Sreeramaswamy, CERN/EP-SFT,\
  Vassil Vassilev, Princeton,\
  Sandro Wenzel, CERN/EP-ALICE,\
+ Tristan Wenzel, CERN/EP-ALICE,\
 
 ## Deprecation and Removal
 

geom/geom/inc/TGeoBoolNode.h

@@ -24,8 +24,8 @@ class TGeoMatrix;
 class TGeoHMatrix;
 
 class TGeoBoolNode : public TObject {
-   static std::atomic<UInt_t> fgInstanceCount; //! source of dense per-object indices
-   UInt_t fIndex{fgInstanceCount++};           //! dense index of this node into the per-thread vector
+   static std::atomic<UInt_t> fgInstanceCount; //! source of monotonic per-object indices
+   UInt_t fIndex{fgInstanceCount++};           //! non-reused index of this node into the per-thread vector
 
 public:
    enum EGeoBoolType {
@@ -39,6 +39,7 @@ class TGeoBoolNode : public TObject {
 
    /// Per-thread scratch state, owned by the calling thread and indexed by this node.
    /// Each thread owns its whole vector, so no two threads ever write the same cache line.
+   /// The vector retains its high-water size until the owning thread exits.
    ThreadData_t &GetThreadData() const
    {
       thread_local std::vector<ThreadData_t> tdata;
@@ -47,6 +48,7 @@ class TGeoBoolNode : public TObject {
       return tdata[fIndex];
    }
    void ClearThreadData() const {}
+   /// No-op: this node allocates its scratch state lazily for every calling thread.
    void CreateThreadData(Int_t) {}
 
 private:

geom/geom/inc/TGeoMatrix.h

@@ -29,6 +29,7 @@ const Double_t kIdentityMatrix[3 * 3] = {1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0,
 const Double_t kUnitScale[3] = {1.0, 1.0, 1.0};
 
 class TGeoHMatrix;
+class TGeoManager;
 
 ////////////////////////////////////////////////////////////////////////////
 //                                                                        //
@@ -96,6 +97,9 @@ class TGeoMatrix : public TNamed {
    virtual void ReflectY(Bool_t leftside, Bool_t rotonly = kFALSE);
    virtual void ReflectZ(Bool_t leftside, Bool_t rotonly = kFALSE);
    virtual void RegisterYourself();
+   /// Register this matrix with an explicit owning geometry manager.
+   /// \param manager Geometry manager that takes ownership of the matrix.
+   void RegisterYourself(TGeoManager *manager);
    void SetDefaultName();
    virtual void SetDx(Double_t) {}
    virtual void SetDy(Double_t) {}
@@ -341,6 +345,9 @@ class TGeoCombiTrans : public TGeoMatrix {
    TGeoMatrix *MakeClone() const override;
    void Multiply(const TGeoMatrix *right);
    void RegisterYourself() override;
+   /// Register this matrix and its rotation with an explicit owning geometry manager.
+   /// \param manager Geometry manager that takes ownership of the transformations.
+   void RegisterYourself(TGeoManager *manager);
    void RotateX(Double_t angle) override;
    void RotateY(Double_t angle) override;
    void RotateZ(Double_t angle) override;

geom/geom/inc/TGeoPatternFinder.h

@@ -24,8 +24,8 @@ class TGeoMatrix;
 
 /// base finder class for patterns. A pattern is specifying a division type
 class TGeoPatternFinder : public TObject {
-   static std::atomic<UInt_t> fgInstanceCount; //! source of dense per-object indices
-   UInt_t fIndex{fgInstanceCount++};           //! dense index of this finder into the per-thread vector
+   static std::atomic<UInt_t> fgInstanceCount; //! source of monotonic per-object indices
+   UInt_t fIndex{fgInstanceCount++};           //! non-reused index of this finder into the per-thread vector
    mutable std::atomic<Int_t> fGeneration{0};  //! bumped whenever the per-thread state must be rebuilt
 
 public:
@@ -41,6 +41,7 @@ class TGeoPatternFinder : public TObject {
 
    /// Per-thread scratch state, owned by the calling thread and indexed by this finder.
    /// Hot path: a TLS read plus an indexed load; the cold rebuild lives in InitThreadSlot().
+   /// The vector retains its high-water size until the owning thread exits.
    ThreadData_t &GetThreadData() const
    {
       thread_local std::vector<ThreadData_t> tdata;
@@ -54,14 +55,18 @@ class TGeoPatternFinder : public TObject {
    /// Invalidate the per-thread data. Each thread rebuilds its own slot lazily on next access,
    /// so no cross-thread reach-in is needed.
    void ClearThreadData() const { fGeneration.fetch_add(1, std::memory_order_release); }
-   /// No-op: per-thread data is allocated lazily, so no provisioning for a fixed thread count
-   /// is required and any number of threads works.
+   /// No-op: this finder allocates its scratch state lazily for every calling thread.
    void CreateThreadData(Int_t) {}
 
 protected:
    void InitThreadSlot(ThreadData_t &td) const;
+   TGeoManager *GetOwnerManager() const { return fVolume ? fVolume->GetGeoManager() : nullptr; }
+   TGeoMatrix *GetOwnerIdentity() const;
 
-   enum EGeoPatternFlags { kPatternReflected = BIT(14), kPatternSpacedOut = BIT(15) };
+   enum EGeoPatternFlags {
+      kPatternReflected = BIT(14),
+      kPatternSpacedOut = BIT(15)
+   };
    Double_t fStep;      // division step length
    Double_t fStart;     // starting point on divided axis
    Double_t fEnd;       // ending point

geom/geom/inc/TGeoPgon.h

@@ -16,31 +16,25 @@
 
 #include <algorithm>
 #include <atomic>
+#include <memory>
+#include <mutex>
 #include <vector>
 
 class TGeoPgon : public TGeoPcon {
-   static std::atomic<UInt_t> fgInstanceCount; //! source of dense per-object indices
-   UInt_t fIndex{fgInstanceCount++};           //! dense index of this shape into the per-thread vector
+   static std::atomic<UInt_t> fgInstanceCount; //! source of monotonic per-object indices
+   UInt_t fIndex{fgInstanceCount++};           //! non-reused index of this shape into the per-thread vector
    mutable std::atomic<Int_t> fGeneration{0};  //! bumped whenever the per-thread state must be rebuilt
 
 public:
    struct ThreadData_t {
       Int_t *fIntBuffer{nullptr};    //![fNedges+4] temporary int buffer array
       Double_t *fDblBuffer{nullptr}; //![fNedges+4] temporary double buffer array
       Int_t fInitGen{-1};            //! generation this slot was last initialized for
-
-      ThreadData_t() = default;
-      ~ThreadData_t();
-      // Owns the two scratch buffers: movable so the slot can live in a resizable vector,
-      // but not copyable.
-      ThreadData_t(ThreadData_t &&other) noexcept;
-      ThreadData_t &operator=(ThreadData_t &&other) noexcept;
-      ThreadData_t(const ThreadData_t &) = delete;
-      ThreadData_t &operator=(const ThreadData_t &) = delete;
    };
 
-   /// Per-thread scratch buffers, owned by the calling thread and indexed by this shape.
+   /// Per-thread non-owning cache of scratch buffers indexed by this shape.
    /// Hot path: a TLS read plus an indexed load; the cold rebuild lives in InitThreadSlot().
+   /// The vector retains its high-water size until the owning thread exits.
    ThreadData_t &GetThreadData() const
    {
       thread_local std::vector<ThreadData_t> tdata;
@@ -51,17 +45,20 @@ class TGeoPgon : public TGeoPcon {
          InitThreadSlot(td);
       return td;
    }
-   /// Invalidate the per-thread data. Each thread rebuilds its own slot lazily on next access.
-   void ClearThreadData() const override { fGeneration.fetch_add(1, std::memory_order_release); }
-   /// No-op: per-thread data is allocated lazily, so no provisioning for a fixed thread count
-   /// is required and any number of threads works.
+   /// Release object-owned scratch buffers and invalidate the non-owning TLS slots.
+   /// Navigation using this shape must be quiescent before this method is called.
+   void ClearThreadData() const override;
+   /// No-op: this shape allocates scratch data lazily for every calling thread.
    void CreateThreadData(Int_t) override {}
 
 protected:
+   struct OwnedThreadData_t;
    void InitThreadSlot(ThreadData_t &td) const;
 
    // data members
-   Int_t fNedges; // number of edges (at least one)
+   Int_t fNedges;                                                      // number of edges (at least one)
+   mutable std::vector<std::unique_ptr<OwnedThreadData_t>> fOwnedData; ///<! Object-owned per-thread buffers
+   mutable std::mutex fOwnedDataMutex;                                 ///<! Protects cold allocation and cleanup
 
    // internal utility methods
    Int_t GetPhiCrossList(const Double_t *point, const Double_t *dir, Int_t istart, Double_t *sphi, Int_t *iphi,

geom/geom/inc/TGeoVolume.h

@@ -317,8 +317,8 @@ class TGeoVolumeMulti : public TGeoVolume {
 ////////////////////////////////////////////////////////////////////////////
 
 class TGeoVolumeAssembly : public TGeoVolume {
-   static std::atomic<UInt_t> fgInstanceCount; //! source of dense per-object indices
-   UInt_t fIndex{fgInstanceCount++};           //! dense index of this assembly into the per-thread vector
+   static std::atomic<UInt_t> fgInstanceCount; //! source of monotonic per-object indices
+   UInt_t fIndex{fgInstanceCount++};           //! non-reused index of this assembly into the per-thread vector
 
 public:
    struct ThreadData_t {
@@ -328,6 +328,7 @@ class TGeoVolumeAssembly : public TGeoVolume {
 
    /// Per-thread scratch state, owned by the calling thread and indexed by this assembly.
    /// Each thread owns its whole vector, so no two threads ever write the same cache line.
+   /// The vector retains its high-water size until the owning thread exits.
    ThreadData_t &GetThreadData() const
    {
       thread_local std::vector<ThreadData_t> tdata;

geom/geom/inc/TGeoXtru.h

@@ -16,13 +16,15 @@
 
 #include <algorithm>
 #include <atomic>
+#include <memory>
+#include <mutex>
 #include <vector>
 
 class TGeoPolygon;
 
 class TGeoXtru : public TGeoBBox {
-   static std::atomic<UInt_t> fgInstanceCount;          //! source of dense per-object indices
-   UInt_t fIndex{fgInstanceCount++};                    //! dense index of this shape into the per-thread vector
+   static std::atomic<UInt_t> fgInstanceCount;          //! source of monotonic per-object indices
+   UInt_t fIndex{fgInstanceCount++};                    //! non-reused index of this shape into the per-thread vector
    mutable std::atomic<Int_t> fGeneration{0};           //! bumped whenever the per-thread state must be rebuilt
    mutable std::atomic<Bool_t> fIllegalChecked{kFALSE}; //! illegal-polygon warning already emitted
 
@@ -34,18 +36,11 @@ class TGeoXtru : public TGeoBBox {
       Double_t *fYc{nullptr};      //![fNvert] current Y positions for polygon vertices
       TGeoPolygon *fPoly{nullptr}; //! polygon defining section shape
       Int_t fInitGen{-1};          //! generation this slot was last initialized for
-
-      ThreadData_t() = default;
-      ~ThreadData_t();
-      // Owns fXc/fYc/fPoly: movable so the slot can live in a resizable vector, not copyable.
-      ThreadData_t(ThreadData_t &&other) noexcept;
-      ThreadData_t &operator=(ThreadData_t &&other) noexcept;
-      ThreadData_t(const ThreadData_t &) = delete;
-      ThreadData_t &operator=(const ThreadData_t &) = delete;
    };
 
-   /// Per-thread scratch state, owned by the calling thread and indexed by this shape.
+   /// Per-thread non-owning cache of scratch state indexed by this shape.
    /// Hot path: a TLS read plus an indexed load; the cold rebuild lives in InitThreadSlot().
+   /// The vector retains its high-water size until the owning thread exits.
    ThreadData_t &GetThreadData() const
    {
       thread_local std::vector<ThreadData_t> tdata;
@@ -56,17 +51,14 @@ class TGeoXtru : public TGeoBBox {
          InitThreadSlot(td);
       return td;
    }
-   /// Invalidate the per-thread data. Each thread rebuilds its own slot lazily on next access.
-   void ClearThreadData() const override
-   {
-      fGeneration.fetch_add(1, std::memory_order_release);
-      fIllegalChecked.store(kFALSE, std::memory_order_relaxed);
-   }
-   /// No-op: per-thread data is allocated lazily, so no provisioning for a fixed thread count
-   /// is required and any number of threads works.
+   /// Release object-owned scratch buffers and invalidate the non-owning TLS slots.
+   /// Navigation using this shape must be quiescent before this method is called.
+   void ClearThreadData() const override;
+   /// No-op: this shape allocates scratch data lazily for every calling thread.
    void CreateThreadData(Int_t) override {}
 
 protected:
+   struct OwnedThreadData_t;
    void InitThreadSlot(ThreadData_t &td) const;
 
    // data members
@@ -79,6 +71,8 @@ class TGeoXtru : public TGeoBBox {
    Double_t *fScale;   //[fNz] array of scale factors (for each Z)
    Double_t *fX0;      //[fNz] array of X offsets (for each Z)
    Double_t *fY0;      //[fNz] array of Y offsets (for each Z)
+   mutable std::vector<std::unique_ptr<OwnedThreadData_t>> fOwnedData; ///<! Object-owned per-thread buffers
+   mutable std::mutex fOwnedDataMutex;                                 ///<! Protects cold allocation and cleanup
 
    TGeoXtru(const TGeoXtru &) = delete;
    TGeoXtru &operator=(const TGeoXtru &) = delete;

geom/geom/src/TGeoManager.cxx

@@ -976,7 +976,9 @@ void TGeoManager::RemoveNavigator(const TGeoNavigator *nav)
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-/// Set maximum number of threads for navigation.
+/// Enable multi-threaded navigation for at most `nthreads` worker threads.
+/// This is still required to make navigator registration and manager state thread-safe;
+/// lazy scratch-data allocation does not replace this setup.
 
 void TGeoManager::SetMaxThreads(Int_t nthreads)
 {

geom/geom/src/TGeoMatrix.cxx

@@ -539,12 +539,20 @@ void TGeoMatrix::ReflectZ(Bool_t, Bool_t) {}
 
 void TGeoMatrix::RegisterYourself()
 {
-   if (!gGeoManager) {
+   RegisterYourself(gGeoManager);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+/// Register the matrix in the given manager, which will become the owner.
+
+void TGeoMatrix::RegisterYourself(TGeoManager *manager)
+{
+   if (!manager) {
       Warning("RegisterYourself", "cannot register without geometry");
       return;
    }
    if (!IsRegistered()) {
-      gGeoManager->RegisterMatrix(this);
+      manager->RegisterMatrix(this);
       SetBit(kGeoRegistered);
    }
 }
@@ -1940,9 +1948,17 @@ void TGeoCombiTrans::Multiply(const TGeoMatrix *right)
 
 void TGeoCombiTrans::RegisterYourself()
 {
-   TGeoMatrix::RegisterYourself();
-   if (fRotation && fRotation->IsRotation())
-      fRotation->RegisterYourself();
+   RegisterYourself(gGeoManager);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+/// Register the matrix and its rotation in the given manager.
+
+void TGeoCombiTrans::RegisterYourself(TGeoManager *manager)
+{
+   TGeoMatrix::RegisterYourself(manager);
+   if (manager && fRotation && fRotation->IsRotation())
+      fRotation->RegisterYourself(manager);
 }
 
 ////////////////////////////////////////////////////////////////////////////////