Merge pull request root-project#33 from Dr15Jones/additionalThreadSaf…

…etyFixes

Additional thread safety fixes

core/base/src/TDirectory.cxx

@@ -244,7 +244,10 @@ TObject *TDirectory::CloneObject(const TObject *obj, Bool_t autoadd /* = kTRUE *
 
    // if no default ctor return immediately (error issued by New())
    char *pobj = (char*)obj->IsA()->New();
-   if (!pobj) return 0;
+   if (!pobj) {
+     Fatal("CloneObject","Failed to create new object");
+     return 0;
+   }
 
    Int_t baseOffset = obj->IsA()->GetBaseClassOffset(TObject::Class());
    if (baseOffset==-1) {
@@ -260,7 +263,10 @@ TObject *TDirectory::CloneObject(const TObject *obj, Bool_t autoadd /* = kTRUE *
    //We are forced to go via the I/O package (ie TBufferFile).
    //Invoking TBufferFile via CINT will automatically load the I/O library
    TBuffer *buffer = (TBuffer*)gROOT->ProcessLine("new TBufferFile(TBuffer::kWrite,10000);");
-   if (!buffer) return 0;
+   if (!buffer) {
+     Fatal("CloneObject","Not able to create a TBuffer!");
+     return 0;
+   }
    buffer->MapObject(obj);  //register obj in map to handle self reference
    const_cast<TObject*>(obj)->Streamer(*buffer);
 

core/base/src/TObject.cxx

@@ -205,8 +205,12 @@ TObject *TObject::Clone(const char *) const
    // This usually means that the object will be appended to the current
    // ROOT directory.
 
-   if (gDirectory) return gDirectory->CloneObject(this);
-   else            return 0;
+   if (gDirectory) {
+     return gDirectory->CloneObject(this);
+   } else {
+     Fatal("Clone","No gDirectory set");
+     return 0;
+   }
 }
 
 //______________________________________________________________________________

core/base/src/TROOT.cxx

@@ -1025,7 +1025,7 @@ TClass *TROOT::FindSTLClass(const char *name, Bool_t load, Bool_t silent) const
    // return a TClass object corresponding to 'name' assuming it is an STL container.
    // In particular we looking for possible alternative name (default template
    // parameter, typedefs template arguments, typedefed name).
-
+   R__LOCKGUARD(gCINTMutex);
    return R__FindSTLClass(name,load,silent,name);
 }
 
@@ -1493,7 +1493,10 @@ TClass *TROOT::LoadClass(const char *requestedname, Bool_t silent) const
 
    if (!dict) {
       // Try to remove the ROOT typedefs
-      resolved = TClassEdit::ResolveTypedef(classname,kTRUE);
+      {
+         R__LOCKGUARD(gCINTMutex);
+	 resolved = TClassEdit::ResolveTypedef(classname,kTRUE);
+      }
       if (resolved != classname) {
          dict = TClassTable::GetDict(resolved.Data());
       } else {

core/meta/inc/TClass.h

@@ -35,7 +35,7 @@
 #endif
 #include <map>
 #include <string>
-#if __cplusplus > 199711L
+#if __cplusplus >= 201103L
 #include <atomic>
 #endif
 class TBaseClass;
@@ -89,7 +89,7 @@ friend class ROOT::TGenericClassInfo;
 private:
 
    mutable TObjArray *fStreamerInfo;    //Array of TVirtualStreamerInfo
-#if __cplusplus > 199711L
+#if __cplusplus >= 201103L
    mutable std::atomic<std::map<std::string, TObjArray*>*> fConversionStreamerInfo; //Array of the streamer infos derived from another class.
 #else
    mutable std::map<std::string, TObjArray*> *fConversionStreamerInfo; //Array of the streamer infos derived from another class.
@@ -115,7 +115,11 @@ friend class ROOT::TGenericClassInfo;
    TString            fContextMenuTitle;//context menu title
    const type_info   *fTypeInfo;        //pointer to the C++ type information.
    ShowMembersFunc_t  fShowMembers;     //pointer to the class's ShowMembers function
+#if __cplusplus >= 201103L
+   mutable std::atomic<void*> fInterShowMembers;//Interpreter call setup for ShowMembers
+#else
    mutable void      *fInterShowMembers;//Interpreter call setup for ShowMembers
+#endif
    TClassStreamer    *fStreamer;        //pointer to streamer function
    TString            fSharedLibs;      //shared libraries containing class code
 
@@ -139,17 +143,18 @@ friend class ROOT::TGenericClassInfo;
    Int_t               fSizeof;         //Sizeof the class.
 
    mutable Int_t      fCanSplit;        //!Indicates whether this class can be split or not.
-   mutable Long_t     fProperty;        //!Property
-#if __cplusplus > 199711L
+#if __cplusplus >= 201103L
+   mutable std::atomic<Long_t> fProperty;        //!Property
    mutable std::atomic<Bool_t> fVersionUsed;     //!Indicates whether GetClassVersion has been called
 #else
+   mutable Long_t     fProperty;        //!Property
    mutable Bool_t     fVersionUsed;     //!Indicates whether GetClassVersion has been called
 #endif
 
    mutable Bool_t     fIsOffsetStreamerSet; //!saved remember if fOffsetStreamer has been set.
    mutable Long_t     fOffsetStreamer;  //!saved info to call Streamer
    Int_t              fStreamerType;    //!cached of the streaming method to use
-#if __cplusplus > 199711L
+#if __cplusplus >= 201103L
    mutable std::atomic<TVirtualStreamerInfo*>  fCurrentInfo;     //!cached current streamer info.
 #else
    mutable TVirtualStreamerInfo     *fCurrentInfo;     //!cached current streamer info.
@@ -185,7 +190,7 @@ friend class ROOT::TGenericClassInfo;
    void StreamerDefault(void *object, TBuffer &b, const TClass *onfile_class) const;
 
    static IdMap_t    *GetIdMap();       //Map from typeid to TClass pointer
-#if __cplusplus > 199711L 
+#if __cplusplus >= 201103L 
    static thread_local ENewType  fgCallingNew;  //Intent of why/how TClass::New() is called
    static std::atomic<Int_t>     fgClassCount;  //provides unique id for a each class
 #else

core/meta/src/TCint.cxx

@@ -2103,8 +2103,9 @@ void *TCint::FindSpecialObject(const char *item, G__ClassInfo *type,
    // This function tries to find the UO in the ROOT files, directories, etc.
    // This functions has been registered by the TCint ctor.
 
-   if (!*prevObj || *assocPtr != gDirectory) {
+   //must protect calls to fgSetOfSpecials and call to G__ClassInfo::Init
    R__LOCKGUARD(gCINTMutex);
+   if (!*prevObj || *assocPtr != gDirectory) {
       *prevObj = gROOT->FindSpecialObject(item, *assocPtr);
       if (!fgSetOfSpecials) fgSetOfSpecials = new std::set<TObject*>;
       if (*prevObj) ((std::set<TObject*>*)fgSetOfSpecials)->insert((TObject*)*prevObj);

core/meta/src/TClass.cxx

@@ -1152,7 +1152,7 @@ TClass::TClass(const TClass& cl) :
   fContextMenuTitle(cl.fContextMenuTitle),
   fTypeInfo(cl.fTypeInfo),
   fShowMembers(cl.fShowMembers),
-  fInterShowMembers(cl.fInterShowMembers),
+  fInterShowMembers(0),
   fStreamer(cl.fStreamer),
   fSharedLibs(cl.fSharedLibs),
   fIsA(cl.fIsA),
@@ -1169,7 +1169,7 @@ TClass::TClass(const TClass& cl) :
   fStreamerFunc(cl.fStreamerFunc),
   fSizeof(cl.fSizeof),
   fCanSplit(cl.fCanSplit),
-  fProperty(cl.fProperty),
+  fProperty(0),
   fVersionUsed(),
   fIsOffsetStreamerSet(cl.fIsOffsetStreamerSet),
   fOffsetStreamer(cl.fOffsetStreamer),
@@ -1276,7 +1276,11 @@ TClass::~TClass()
 
    fIsOffsetStreamerSet=kFALSE;
 
+#if __cplusplus >= 201103L
+   if (fInterShowMembers) gCint->CallFunc_Delete(fInterShowMembers.load());
+#else
    if (fInterShowMembers) gCint->CallFunc_Delete(fInterShowMembers);
+#endif
 
    if ( fIsA ) delete fIsA;
 
@@ -1863,20 +1867,24 @@ Bool_t TClass::CallShowMembers(void* obj, TMemberInspector &insp,
          //
 
          if (!fInterShowMembers) {
-            CallFunc_t* ism = gCint->CallFunc_Factory();
-            Long_t offset = 0;
 
             R__LOCKGUARD2(gCINTMutex);
-            gCint->CallFunc_SetFuncProto(ism,fClassInfo,"ShowMembers", "TMemberInspector&", &offset);
-            if (fIsOffsetStreamerSet && offset != fOffsetStreamer) {
-               Error("CallShowMembers", "Logic Error: offset for Streamer() and ShowMembers() differ!");
-               fInterShowMembers = 0;
-               return kFALSE;
-            }
-
-            fInterShowMembers = ism;
+	    if(!fInterShowMembers) {
+	       CallFunc_t* ism = gCint->CallFunc_Factory();
+	       Long_t offset = 0;
+
+	       gCint->CallFunc_SetFuncProto(ism,fClassInfo,"ShowMembers", "TMemberInspector&", &offset);
+	       if (fIsOffsetStreamerSet && offset != fOffsetStreamer) {
+		  Error("CallShowMembers", "Logic Error: offset for Streamer() and ShowMembers() differ!");
+		  fInterShowMembers = 0;
+		  return kFALSE;
+	       }
+
+	       fInterShowMembers = ism;
+	    }
          }
-         if (!gCint->CallFunc_IsValid(fInterShowMembers)) {
+	 void* interShowMembers = fInterShowMembers;
+         if (!gCint->CallFunc_IsValid(interShowMembers)) {
             if (strcmp(GetName(), "string") == 0) {
                // For std::string we know that we do not have a ShowMembers
                // function and that it's okay.
@@ -1887,10 +1895,10 @@ Bool_t TClass::CallShowMembers(void* obj, TMemberInspector &insp,
             return kFALSE;
          } else {
             R__LOCKGUARD2(gCINTMutex);
-            gCint->CallFunc_ResetArg(fInterShowMembers);
-            gCint->CallFunc_SetArg(fInterShowMembers,(Long_t) &insp);
+            gCint->CallFunc_ResetArg(interShowMembers);
+            gCint->CallFunc_SetArg(interShowMembers,(Long_t) &insp);
             void* address = (void*) (((Long_t) obj) + fOffsetStreamer);
-            gCint->CallFunc_Exec((CallFunc_t*)fInterShowMembers,address);
+            gCint->CallFunc_Exec((CallFunc_t*)interShowMembers,address);
             return kTRUE;
          }
       } else if (TVirtualStreamerInfo* sinfo = GetStreamerInfo()) {
@@ -3982,6 +3990,8 @@ void *TClass::New(ENewType defConstructor) const
       // Register the object for special handling in the destructor.
       if (p) {
          RegisterAddressInRepository("New",p,this);
+      } else {
+	Error("New", "Failed to construct class '%s' using streamer info", GetName());
       }
    } else {
       Error("New", "This cannot happen!");
@@ -4786,8 +4796,6 @@ Long_t TClass::Property() const
 
    if (fClassInfo) {
 
-      kl->fProperty = gCint->ClassInfo_Property(fClassInfo);
-
       if (!gCint->ClassInfo_HasMethod(fClassInfo,"Streamer") ||
           !gCint->ClassInfo_IsValidMethod(fClassInfo,"Streamer","TBuffer&",&dummy) ) {
 
@@ -4815,6 +4823,9 @@ 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 = gCint->ClassInfo_Property(fClassInfo);
 
    } else {
 

core/meta/src/TFunction.cxx

@@ -23,7 +23,7 @@
 #include "TROOT.h"
 #include "TInterpreter.h"
 #include "Strlen.h"
-
+#include "TVirtualMutex.h"
 
 ClassImp(TFunction)
 
@@ -48,6 +48,7 @@ TFunction::TFunction(const TFunction &orig) : TDictionary(orig)
    // Copy operator.
 
    if (orig.fInfo) {
+      R__LOCKGUARD2(gCINTMutex);
       fInfo = gCint->MethodInfo_FactoryCopy(orig.fInfo);
       fMangledName = gCint->MethodInfo_GetMangledName(fInfo);
    } else
@@ -61,6 +62,7 @@ TFunction& TFunction::operator=(const TFunction &rhs)
    // Assignment operator.
 
    if (this != &rhs) {
+      R__LOCKGUARD2(gCINTMutex);
       gCint->MethodInfo_Delete(fInfo);
       if (fMethodArgs) fMethodArgs->Delete();
       delete fMethodArgs;
@@ -81,6 +83,7 @@ TFunction::~TFunction()
 {
    // TFunction dtor deletes adopted CINT MethodInfo.
 
+   R__LOCKGUARD2(gCINTMutex);
    gCint->MethodInfo_Delete(fInfo);
 
    if (fMethodArgs) fMethodArgs->Delete();
@@ -101,7 +104,7 @@ TObject *TFunction::Clone(const char *newname) const
 void TFunction::CreateSignature()
 {
    // Using the CINT method arg information to create a complete signature string.
-
+   R__LOCKGUARD2(gCINTMutex);
    gCint->MethodInfo_CreateSignature(fInfo, fSignature);
 }
 
@@ -134,7 +137,7 @@ TList *TFunction::GetListOfMethodArgs()
 const char *TFunction::GetReturnTypeName() const
 {
    // Get full type description of function return type, e,g.: "class TDirectory*".
-
+   R__LOCKGUARD2(gCINTMutex);
    if (gCint->MethodInfo_Type(fInfo) == 0) return "Unknown";
    return gCint->MethodInfo_TypeName(fInfo);
 }
@@ -147,7 +150,7 @@ std::string TFunction::GetReturnTypeNormalizedName() const
    // which include Double32_t, Float16_t, [U]Long64_t and std::string.  It
    // also has std:: removed [This is subject to change].
    //
-
+   R__LOCKGUARD2(gCINTMutex);
    if (gCint->MethodInfo_Type(fInfo) == 0) return "Unknown";
    return gCint->MethodInfo_TypeNormalizedName(fInfo);
 }
@@ -209,8 +212,9 @@ const char *TFunction::GetPrototype() const
    // Returns the prototype of a function as defined by CINT, or 0 in
    // case of error.
 
-   if (fInfo)
+   if (fInfo) {
+      R__LOCKGUARD2(gCINTMutex);
       return gCint->MethodInfo_GetPrototype(fInfo);
-   else
+   } else
       return 0;
 }

core/meta/src/TStreamerElement.cxx

@@ -198,7 +198,11 @@ TStreamerElement::TStreamerElement(const char *name, const char *title, Int_t of
    fNewType     = fType;
    fArrayDim    = 0;
    fArrayLength = 0;
-   fTypeName    = TClassEdit::ResolveTypedef(typeName);
+   {
+      //must protect call into the interpreter
+      R__LOCKGUARD2(gCINTMutex);
+      fTypeName    = TClassEdit::ResolveTypedef(typeName);
+   }
    fStreamer    = 0;
    fClassObject = (TClass*)(-1);
    fNewClass    = 0;

hist/hist/src/TProfileHelper.h

@@ -438,6 +438,9 @@ T* TProfileHelper::RebinAxis(T* p, Double_t x, TAxis *axis)
 
    //save a copy of this histogram
    T *hold = (T*)p->Clone();
+   if(nullptr ==hold) {
+     Fatal("RebinAxis","Call to Clone returned a null");
+   }
    hold->SetDirectory(0);
    //set new axis limits
    axis->SetLimits(xmin,xmax);