Merge pull request #60 from Dr15Jones/addPhilippesThreadChanges

Add Philippe Canal's initial TClass thread-safety changes

core/meta/inc/TClass.h

@@ -227,10 +227,11 @@ friend class TProtoClass;
    mutable std::atomic<Long_t> fProperty; //!Property
    mutable Long_t     fClassProperty;     //!C++ Property of the class (is abstract, has virtual table, etc.)
 
-           Bool_t     fHasRootPcmInfo : 1;      //!Whether info was loaded from a root pcm.
-   mutable Bool_t     fCanLoadClassInfo : 1;    //!Indicates whether the ClassInfo is supposed to be available.
-   mutable Bool_t     fIsOffsetStreamerSet : 1; //!saved remember if fOffsetStreamer has been set.
-   mutable std::atomic<Bool_t> fVersionUsed;     //!Indicates whether GetClassVersion has been called
+           // fHasRootPcmInfo needs to be atomic as long as GetListOfBases needs to modify it.
+           std::atomic<Bool_t> fHasRootPcmInfo;      //!Whether info was loaded from a root pcm.
+   mutable std::atomic<Bool_t> fCanLoadClassInfo;    //!Indicates whether the ClassInfo is supposed to be available.
+   mutable std::atomic<Bool_t> fIsOffsetStreamerSet; //!saved remember if fOffsetStreamer has been set.
+   mutable std::atomic<Bool_t> fVersionUsed;         //!Indicates whether GetClassVersion has been called
 
    mutable Long_t     fOffsetStreamer;  //!saved info to call Streamer
    Int_t              fStreamerType;    //!cached of the streaming method to use
@@ -281,7 +282,7 @@ friend class TProtoClass;
    static TDeclNameRegistry fNoInfoOrEmuOrFwdDeclNameRegistry; // Store the decl names of the forwardd and no info instances
    static Bool_t HasNoInfoOrEmuOrFwdDeclaredDecl(const char*);
 
-   // Internal status bits
+   // Internal status bits, set and reset only during initialization and thus under the protection of the global lock.
    enum { kLoading = BIT(14), kUnloading = BIT(14) };
    // Internal streamer type.
    enum EStreamerType {kDefault=0, kEmulatedStreamer=1, kTObject=2, kInstrumented=4, kForeign=8, kExternal=16};

core/meta/src/TClass.cxx

@@ -2037,11 +2037,11 @@ void TClass::CalculateStreamerOffset() const
       // gets allocated on the heap and not in the mapped file.
 
       TMmallocDescTemp setreset;
-      fIsOffsetStreamerSet = kTRUE;
       fOffsetStreamer = const_cast<TClass*>(this)->GetBaseClassOffsetRecurse(TObject::Class());
       if (fStreamerType == kTObject) {
          fStreamerImpl = &TClass::StreamerTObjectInitialized;
       }
+      fIsOffsetStreamerSet = kTRUE;
    }
 }
 
@@ -3285,13 +3285,22 @@ TList *TClass::GetListOfBases()
    if (!fBase) {
       if (fCanLoadClassInfo) {
          if (fState == kHasTClassInit) {
+
+            R__LOCKGUARD(gInterpreterMutex);
+            // NOTE: Add test to prevent redo if another thread has already done the work.
+            // if (!fHasRootPcmInfo) {
+
             // The bases are in our ProtoClass; we don't need the class info.
             TProtoClass *proto = TClassTable::GetProtoNorm(GetName());
             if (proto && proto->FillTClass(this)) {
+               // Not sure this code is still needed
+               // R__ASSERT(kFALSE);
+
                fHasRootPcmInfo = kTRUE;
             }
          }
-         if (!fHasRootPcmInfo) {
+         // We test again on fCanLoadClassInfo has another thread may have executed it.
+         if (!fHasRootPcmInfo && !fCanLoadClassInfo) {
             LoadClassInfo();
          }
       }
@@ -3329,9 +3338,15 @@ TList *TClass::GetListOfDataMembers(Bool_t load /* = kTRUE */)
 
    if (!fData) {
       if (fCanLoadClassInfo && fState == kHasTClassInit) {
+         // NOTE: Add test to prevent redo if another thread has already done the work.
+         // if (!fHasRootPcmInfo) {
+
          // The members are in our ProtoClass; we don't need the class info.
          TProtoClass *proto = TClassTable::GetProtoNorm(GetName());
          if (proto && proto->FillTClass(this)) {
+            // Not sure this code is still needed
+            // R__ASSERT(kFALSE);
+
             fHasRootPcmInfo = kTRUE;
             return fData;
          }
@@ -3885,7 +3900,7 @@ TMethod *TClass::GetMethodAny(const char *method)
    // of the class.
 
    if (!HasInterpreterInfo()) return 0;
-   return (TMethod*) GetListOfMethods()->FindObject(method);
+   return (TMethod*) GetMethodList()->FindObject(method);
 }
 
 //______________________________________________________________________________
@@ -5219,10 +5234,12 @@ void TClass::LoadClassInfo() const
    // Try to load the classInfo (it may require parsing the header file
    // and/or loading data from the clang pcm).
 
-   R__ASSERT(fCanLoadClassInfo);
-
    R__LOCKGUARD(gInterpreterMutex);
 
+   // If another thread executed LoadClassInfo at about the same time
+   // as this thread return early since the work was done.
+   if (!fCanLoadClassInfo) return;
+
    gInterpreter->AutoParse(GetName());
    if (!fClassInfo) gInterpreter->SetClassInfo(const_cast<TClass*>(this));   // sets fClassInfo pointer
    if (!gInterpreter->IsAutoParsingSuspended()) {
@@ -5465,10 +5482,10 @@ Long_t TClass::Property() const
          kl->fStreamerType  = kExternal;
          kl->fStreamerImpl  = &TClass::StreamerExternal;
       }
-      //must set this last since other threads may read fProperty
-      // and think all test bits have been properly set
-      kl->fProperty = gCling->ClassInfo_Property(fClassInfo);
       kl->fClassProperty = gCling->ClassInfo_ClassProperty(GetClassInfo());
+      // Must set this last since other threads may read fProperty
+      // and think all test bits have been properly set.
+      kl->fProperty = gCling->ClassInfo_Property(fClassInfo);
 
    } else {
 
@@ -5479,6 +5496,8 @@ Long_t TClass::Property() const
 
       kl->fStreamerType |= kEmulatedStreamer;
       kl->SetStreamerImpl();
+      // fProperty was *not* set so that it can be forced to be recalculated
+      // next time.
       return 0;
    }