Merge pull request root-project#4 from ktf/fix-mt-io

Fix multithreading issues in ROOT I/O
Dr15Jones · Feb 26, 2014 · 7f9948f · 7f9948f
2 parents 7925282 + 44e5441
commit 7f9948f
Show file tree

Hide file tree

Showing 49 changed files with 678 additions and 264 deletions.
diff --git a/cint/cintex/src/Cintex.cxx b/cint/cintex/src/Cintex.cxx
@@ -25,6 +25,8 @@
 #include <iostream>
 
 #include "TROOT.h"
+#include "TInterpreter.h" //gCINTMutex
+#include "TVirtualMutex.h"
 
 using namespace ROOT::Reflex;
 using namespace ROOT::Cintex;
@@ -187,6 +189,7 @@ namespace ROOT {
       }
 
       void Callback::operator () ( const Type& t ) {
+         R__LOCKGUARD2(gCINTMutex);
          ArtificialSourceFile asf;
          int autoload = G__set_class_autoloading(0); // To avoid recursive loads
          if ( t.IsClass() || t.IsStruct() ) {
@@ -205,6 +208,7 @@ namespace ROOT {
       }
 
       void Callback::operator () ( const Member& m ) {
+         R__LOCKGUARD2(gCINTMutex);
          ArtificialSourceFile asf;
          int autoload = G__set_class_autoloading(0); // To avoid recursive loads
          if ( m.IsFunctionMember() ) {

diff --git a/cint/cintex/src/ROOTClassEnhancer.cxx b/cint/cintex/src/ROOTClassEnhancer.cxx
@@ -21,6 +21,7 @@
 #include "TClassStreamer.h"
 #include "TCollectionProxyInfo.h"
 #include "TVirtualCollectionProxy.h"
+#include "TVirtualMutex.h"
 #include "TMemberInspector.h"
 #include "RVersion.h"
 #include "Reflex/Reflex.h"
@@ -35,6 +36,9 @@
 #include <sstream>
 #include <memory>
 #include <string>
+#if __cplusplus > 199711L
+#include <atomic>
+#endif
 
 #if ROOT_VERSION_CODE >= ROOT_VERSION(5,1,1)
 #include "TVirtualIsAProxy.h"
@@ -44,6 +48,8 @@ using namespace ROOT::Reflex;
 using namespace ROOT::Cintex;
 using namespace std;
 
+static TVirtualMutex* gCintexMutex = 0;
+
 namespace ROOT { namespace Cintex {
 
    class IsAProxy;
@@ -52,7 +58,11 @@ namespace ROOT { namespace Cintex {
 
       Type                     fType;
       string                   fName;
+#if __cplusplus > 199711L
+     std::atomic<TClass*>      fTclass;
+#else
       TClass*                  fTclass;
+#endif
       TClass*                  fLastClass;
       std::map<const std::type_info*,TClass*> fSub_types;
       const  std::type_info*   fLastType;
@@ -174,7 +184,10 @@ namespace ROOT { namespace Cintex {
       // Constructor.
       fType = CleanType(t);
       fName = CintName(fType);
-      rootEnhancerInfos().push_back(this);
+      {
+	R__LOCKGUARD2(gCintexMutex);
+	rootEnhancerInfos().push_back(this);
+      }
       fMyType = &t.TypeInfo();
       fIsVirtual = TypeGet().IsVirtual();
       fClassInfo = 0;
@@ -361,37 +374,46 @@ namespace ROOT { namespace Cintex {
       if ( ! obj || ! fIsVirtual )  {
          return Tclass();
       }
-      else  {
-         // Avoid the case that the first word is a virtual_base_offset_table instead of
-         // a virtual_function_table
-         long Offset = **(long**)obj;
-         if ( Offset == 0 ) return Tclass();
 
-         DynamicStruct_t* p = (DynamicStruct_t*)obj;
-         const std::type_info& typ = typeid(*p);
+      // Avoid the case that the first word is a virtual_base_offset_table instead of
+      // a virtual_function_table
+      long Offset = **(long**)obj;
+      if ( Offset == 0 ) return Tclass();
 
-         if ( &typ == fMyType )  {
-            return Tclass();
-         }
-         else if ( &typ == fLastType )  {
+      DynamicStruct_t* p = (DynamicStruct_t*)obj;
+      const std::type_info& typ = typeid(*p);
+
+      if ( &typ == fMyType )  {
+	return Tclass();
+      }
+      {
+	 R__LOCKGUARD2(gCintexMutex);
+         if ( &typ == fLastType )  {
             return fLastClass;
          }
+
          // Check if TypeNth is already in sub-class cache
-         else if ( 0 != (fLastClass=fSub_types[&typ]) )  {
+	 TClass* findClass = fSub_types[&typ];
+         if ( 0 != findClass )  {
+ 	    fLastClass = findClass;
             fLastType = &typ;
+	    return fLastClass;
          }
-         // Last resort: lookup root class
-         else   {
-            std::string nam;
-            Type t = Type::ByTypeInfo(typ);
-            if (t) nam = CintName(t);
-            else   nam = CintName(Tools::Demangle(typ));
-            fLastClass = ROOT::GetROOT()->GetClass(nam.c_str());
-            fSub_types[fLastType=&typ] = fLastClass;
-         }
+      }
+      // Last resort: lookup root class
+      TClass* returnValue;
+      std::string nam;
+      Type t = Type::ByTypeInfo(typ);
+      if (t) nam = CintName(t);
+      else   nam = CintName(Tools::Demangle(typ));
+      returnValue = ROOT::GetROOT()->GetClass(nam.c_str());
+      {
+         R__LOCKGUARD2(gCintexMutex);
+	 fLastClass = returnValue;
+	 fSub_types[fLastType=&typ] = fLastClass;
       }
       //std::cout << "Cintex: IsA:" << TypeNth.Name(SCOPED) << " dynamic:" << dtype.Name(SCOPED) << std::endl;
-      return fLastClass;
+      return returnValue;
    }
 
    TClass* ROOTClassEnhancerInfo::Default_CreateClass( Type typ, ROOT::TGenericClassInfo* info)  {

diff --git a/cint/reflex/python/genreflex/gendict.py b/cint/reflex/python/genreflex/gendict.py
@@ -2654,6 +2654,8 @@ def ClassDefImplementation(selclasses, self) :
   returnValue += '#endif\n'
   returnValue += '#include "TClass.h"\n'
   returnValue += '#include "TMemberInspector.h"\n'
+  returnValue += '#include "TInterpreter.h"\n'
+  returnValue += '#include "TVirtualMutex.h"\n'
   returnValue += '#include "RtypesImp.h"\n' # for GenericShowMembers etc
   returnValue += '#include "TIsAProxy.h"\n'
   haveClassDef = 0
@@ -2725,8 +2727,11 @@ def ClassDefImplementation(selclasses, self) :
         specclname = clname
 
       returnValue += template + 'TClass* ' + specclname + '::Class() {\n'
-      returnValue += '   if (!fgIsA)\n'
-      returnValue += '      fgIsA = TClass::GetClass("' + clname[2:] + '");\n'
+      returnValue += '   if (!fgIsA) {\n'
+      returnValue += '      R__LOCKGUARD2(gCINTMutex);'
+      returnValue += '      if (!fgIsA)\n'
+      returnValue += '         fgIsA = TClass::GetClass("' + clname[2:] + '");\n'
+      returnValue += '   }\n'
       returnValue += '   return fgIsA;\n'
       returnValue += '}\n'
       returnValue += template + 'const char * ' + specclname + '::Class_Name() {return "' + clname[2:]  + '";}\n'
@@ -2814,7 +2819,7 @@ def ClassDefImplementation(selclasses, self) :
       returnValue += '      b.WriteClassBuffer(' + clname  + '::Class(),this);\n'
       returnValue += '   }\n'
       returnValue += '}\n'
-      returnValue += template + 'TClass* ' + specclname + '::fgIsA = 0;\n'
+      returnValue += template + 'atomic_TClass_ptr ' + specclname + '::fgIsA(0);\n'
       returnValue += namespacelevel * '}' + '\n'
     elif derivesFromTObject :
       # no fgIsA etc members but derives from TObject!

diff --git a/core/base/inc/Rtypes.h b/core/base/inc/Rtypes.h
@@ -34,7 +34,9 @@
 #include <string.h>
 #include <snprintf.h>   // part of stdio.h on systems that have it
 #include <strlcpy.h>    // part of string.h on systems that have it
-
+#if __cplusplus > 199711L
+#include <atomic>
+#endif
 
 
 //---- forward declared class types --------------------------------------------
@@ -268,12 +270,17 @@ namespace ROOT {
 #include "TGenericClassInfo.h"
 #endif
 
+#if __cplusplus > 199711L
+typedef std::atomic<TClass*> atomic_TClass_ptr;
+#else
+typedef TClass* atomic_TClass_ptr;
+#endif
 // Common part of ClassDef definition.
 // DeclFileLine() is not part of it since CINT uses that as trigger for
 // the class comment string.
 #define _ClassDef_(name,id) \
 private: \
-   static TClass *fgIsA; \
+   static atomic_TClass_ptr fgIsA; \
 public: \
    static TClass *Class(); \
    static const char *Class_Name(); \
@@ -290,7 +297,7 @@ public: \
 // Version without any virtual functions.
 #define _ClassDefNV_(name,id) \
 private: \
-static TClass *fgIsA; \
+static atomic_TClass_ptr fgIsA; \
 public: \
 static TClass *Class(); \
 static const char *Class_Name(); \

diff --git a/core/base/inc/TROOT.h b/core/base/inc/TROOT.h
@@ -241,7 +241,7 @@ friend class TCintWithCling;
    Long_t            ProcessLine(const char *line, Int_t *error = 0);
    Long_t            ProcessLineSync(const char *line, Int_t *error = 0);
    Long_t            ProcessLineFast(const char *line, Int_t *error = 0);
-   Bool_t            ReadingObject() const { /* Deprecated (will be removed in next release) */ return fReadingObject; }
+   Bool_t            ReadingObject() const;
    void              RefreshBrowsers();
    void              RemoveClass(TClass *);
    void              Reset(Option_t *option="");
@@ -258,7 +258,7 @@ friend class TCintWithCling;
    void              SetEscape(Bool_t flag = kTRUE) { fEscape = flag; }
    void              SetLineIsProcessing() { fLineIsProcessing++; }
    void              SetLineHasBeenProcessed() { if (fLineIsProcessing) fLineIsProcessing--; }
-   void              SetReadingObject(Bool_t flag = kTRUE) { fReadingObject = flag; }
+   void              SetReadingObject(Bool_t flag = kTRUE);
    void              SetMustClean(Bool_t flag = kTRUE) { fMustClean=flag; }
    void              SetSelectedPrimitive(const TObject *obj) { fPrimitive = obj; }
    void              SetSelectedPad(TVirtualPad *pad) { fSelectPad = pad; }

diff --git a/core/base/inc/TStorage.h b/core/base/inc/TStorage.h
@@ -30,8 +30,6 @@ typedef void *(*ReAllocFun_t)(void*, size_t);
 typedef void *(*ReAllocCFun_t)(void*, size_t, size_t);
 typedef char *(*ReAllocCharFun_t)(char*, size_t, size_t);
 
-/*Magic number written to memory after TStorage::ObjectAlloc */
-#define TSTORAGEMEMVALUE 0x99999999
 
 class TStorage {
 
@@ -42,6 +40,8 @@ class TStorage {
    static ReAllocFun_t   fgReAllocHook;        // custom ReAlloc
    static ReAllocCFun_t  fgReAllocCHook;       // custom ReAlloc with length check
    static Bool_t         fgHasCustomNewDelete; // true if using ROOT's new/delete
+   static const UInt_t   kObjectAllocMemValue = 0x99999999;
+                                               // magic number for ObjectAlloc
 
 public:
    virtual ~TStorage() { }
@@ -77,10 +77,14 @@ class TStorage {
    static void   AddToHeap(ULong_t begin, ULong_t end);
    static Bool_t IsOnHeap(void *p);
 
+   static Bool_t FilledByObjectAlloc(UInt_t* member);
+
    ClassDef(TStorage,0)  //Storage manager class
 };
 
 #ifndef WIN32
+inline Bool_t TStorage::FilledByObjectAlloc(UInt_t *member) { return *member == kObjectAllocMemValue; }
+
 inline size_t TStorage::GetMaxBlockSize() { return fgMaxBlockSize; }
 
 inline void TStorage::SetMaxBlockSize(size_t size) { fgMaxBlockSize = size; }

diff --git a/core/base/inc/TSystem.h b/core/base/inc/TSystem.h
@@ -316,6 +316,10 @@ class TSystem : public TNamed {
    TSeqCollection  *fCompiled;         //List of shared libs from compiled macros to be deleted
    TSeqCollection  *fHelpers;          //List of helper classes for alternative file/directory access
 
+#if __cplusplus > 199711L
+   static thread_local TString   fgLastErrorString;  //Last system error message
+#endif
+
    TSystem               *FindHelper(const char *path, void *dirptr = 0);
    virtual Bool_t         ConsistentWith(const char *path, void *dirptr = 0);
    virtual const char    *ExpandFileName(const char *fname);
@@ -340,7 +344,11 @@ class TSystem : public TNamed {
    virtual void            SetProgname(const char *name);
    virtual void            SetDisplay();
    void                    SetErrorStr(const char *errstr);
+#if __cplusplus > 199711L
+   const char             *GetErrorStr() const { return fgLastErrorString; }
+#else
    const char             *GetErrorStr() const { return fLastErrorString; }
+#endif
    virtual const char     *GetError();
    void                    RemoveOnExit(TObject *obj);
    virtual const char     *HostName();

diff --git a/core/base/src/TError.cxx b/core/base/src/TError.cxx
@@ -62,10 +62,8 @@ static void DebugPrint(const char *fmt, ...)
 {
    // Print debugging message to stderr and, on Windows, to the system debugger.
 
-   static Int_t buf_size = 2048;
-   static char *buf = 0;
-
-   R__LOCKGUARD2(gErrorMutex);
+   static thread_local Int_t buf_size = 2048;
+   static thread_local char *buf = 0;
 
    va_list ap;
    va_start(ap, fmt);
@@ -90,6 +88,8 @@ static void DebugPrint(const char *fmt, ...)
    }
    va_end(ap);
 
+   R__LOCKGUARD2(gErrorMutex);
+
    fprintf(stderr, "%s", buf);
 
 #ifdef WIN32
@@ -197,10 +197,9 @@ void ErrorHandler(Int_t level, const char *location, const char *fmt, va_list ap
 {
    // General error handler function. It calls the user set error handler.
 
-   R__LOCKGUARD2(gErrorMutex);
 
-   static Int_t buf_size = 2048;
-   static char *buf = 0;
+   static thread_local Int_t buf_size = 2048;
+   static thread_local char *buf = 0;
 
    int vc = 0;
    va_list sap;
@@ -233,9 +232,10 @@ void ErrorHandler(Int_t level, const char *location, const char *fmt, va_list ap
       va_end(ap);
 
    char *bp;
-   if (level >= kSysError && level < kFatal)
+   if (level >= kSysError && level < kFatal) {
+      R__LOCKGUARD2(gErrorMutex);
       bp = Form("%s (%s)", buf, gSystem->GetError());
-   else
+   } else
       bp = buf;
 
    if (level != kFatal)

diff --git a/core/base/src/TObject.cxx b/core/base/src/TObject.cxx
@@ -62,7 +62,7 @@ Bool_t TObject::fgObjectStat = kTRUE;
 ClassImp(TObject)
 
 //______________________________________________________________________________
-TObject::TObject() : fUniqueID(0) //Need to leave FBits unset
+TObject::TObject() : fBits(kNotDeleted) //Need to leave FUniqueID unset
 {
    // TObject constructor. It sets the two data words of TObject to their
    // initial values. The unique ID is set to 0 and the status word is
@@ -71,11 +71,10 @@ TObject::TObject() : fUniqueID(0) //Need to leave FBits unset
    // (see TEnv) the object is added to the global TObjectTable for
    // bookkeeping.
 
-  if(fBits == TSTORAGEMEMVALUE) {
-    fBits = kNotDeleted|kIsOnHeap;
-  } else {
-    fBits = kNotDeleted;
-  }
+   if (TStorage::FilledByObjectAlloc(&fUniqueID)) 
+      fBits |= kIsOnHeap;
+
+   fUniqueID = 0;
 
    if (fgObjectStat) TObjectTable::AddObj(this);
 }
@@ -85,16 +84,19 @@ TObject::TObject(const TObject &obj)
 {
    // TObject copy ctor.
 
-   fUniqueID = obj.fUniqueID;  // when really unique don't copy
+   fBits = obj.fBits;
 
-   if (fBits == TSTORAGEMEMVALUE) 
-      fBits = obj.fBits | kIsOnHeap;
+   if (TStorage::FilledByObjectAlloc(&fUniqueID))
+      fBits |= kIsOnHeap;
    else
-      fBits = obj.fBits & ~kIsOnHeap;
+      fBits &= ~kIsOnHeap;
 
    fBits &= ~kIsReferenced;
    fBits &= ~kCanDelete;
 
+   //Set only after used in above call
+   fUniqueID = obj.fUniqueID;  // when really unique don't copy
+
    if (fgObjectStat) TObjectTable::AddObj(this);
 }