Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thread-safety fix for TClass::New #2095

Closed
Dr15Jones opened this issue Jan 20, 2014 · 1 comment
Closed

Thread-safety fix for TClass::New #2095

Dr15Jones opened this issue Jan 20, 2014 · 1 comment

Comments

@Dr15Jones
Copy link
Contributor

I have an another patch for ROOT to fix an issue found by helgrind. The pull request to ROOT can be found here: root-project/root#10

The diff from the pull request is given below:

diff --git a/core/meta/src/TClass.cxx b/core/meta/src/TClass.cxx
index 12a9dbc..bbc76b3 100644
--- a/core/meta/src/TClass.cxx
+++ b/core/meta/src/TClass.cxx
@@ -3925,8 +3925,9 @@ void *TClass::New(ENewType defConstructor) const
       // FIXME: Partial Answer: Is this because we may never actually deregister them???

       Bool_t statsave = GetObjectStat();
-      SetObjectStat(kFALSE);
-
+      if(statsave) {
+   SetObjectStat(kFALSE);
+      }
       TVirtualStreamerInfo* sinfo = GetStreamerInfo();
       if (!sinfo) {
          Error("New", "Cannot construct class '%s' version %d, no streamer info available!", GetName(), fClassVersion);
@@ -3939,7 +3940,9 @@ void *TClass::New(ENewType defConstructor) const

       // FIXME: Mistake?  See note above at the GetObjectStat() call.
       // Allow TObject's to be registered again.
-      SetObjectStat(statsave);
+      if(statsave) {
+   SetObjectStat(statsave);
+      }

       // Register the object for special handling in the destructor.
       if (p) {
@@ -4008,7 +4011,9 @@ void *TClass::New(void *arena, ENewType defConstructor) const
       // Do not register any TObject's that we create
       // as a result of creating this object.
       Bool_t statsave = GetObjectStat();
-      SetObjectStat(kFALSE);
+      if(statsave) {
+   SetObjectStat(kFALSE);
+      }

       TVirtualStreamerInfo* sinfo = GetStreamerInfo();
       if (!sinfo) {
@@ -4022,7 +4027,9 @@ void *TClass::New(void *arena, ENewType defConstructor) const

       // ???BUG???
       // Allow TObject's to be registered again.
-      SetObjectStat(statsave);
+      if(statsave) {
+   SetObjectStat(statsave);
+      }

       // Register the object for special handling in the destructor.
       if (p) {
@@ -4092,7 +4099,9 @@ void *TClass::NewArray(Long_t nElements, ENewType defConstructor) const
       // Do not register any TObject's that we create
       // as a result of creating this object.
       Bool_t statsave = GetObjectStat();
-      SetObjectStat(kFALSE);
+      if(statsave) {
+   SetObjectStat(kFALSE);
+      }

       TVirtualStreamerInfo* sinfo = GetStreamerInfo();
       if (!sinfo) {
@@ -4106,7 +4115,9 @@ void *TClass::NewArray(Long_t nElements, ENewType defConstructor) const

       // ???BUG???
       // Allow TObject's to be registered again.
-      SetObjectStat(statsave);
+      if(statsave) {
+   SetObjectStat(statsave);
+      }

       // Register the object for special handling in the destructor.
       if (p) {
@@ -4175,7 +4186,9 @@ void *TClass::NewArray(Long_t nElements, void *arena, ENewType defConstructor) c
       // Do not register any TObject's that we create
       // as a result of creating this object.
       Bool_t statsave = GetObjectStat();
-      SetObjectStat(kFALSE);
+      if(statsave) {
+   SetObjectStat(kFALSE);
+      }

       TVirtualStreamerInfo* sinfo = GetStreamerInfo();
       if (!sinfo) {
@@ -4189,7 +4202,9 @@ void *TClass::NewArray(Long_t nElements, void *arena, ENewType defConstructor) c

       // ???BUG???
       // Allow TObject's to be registered again.
-      SetObjectStat(statsave);
+      if(statsave) {
+   SetObjectStat(statsave);
+      }

       if (fStreamerType & kEmulated) {
          // We always register emulated objects, we need to always
@ktf
Copy link
Contributor

ktf commented Jan 20, 2014

@nclopezo also for this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants