Do not create an extra default instance of T when constructing a ThreadLocal<T>.

git-svn-id: http://googletest.googlecode.com/svn/trunk@732 861a406c-534a-0410-8894-cb66d6ee9925

2 files changed, 87 insertions, 42 deletions

diff --git a/include/gtest/internal/gtest-port.h b/include/gtest/internal/gtest-port.h
index f6ed4d0..936dfd5 100644
--- a/include/gtest/internal/gtest-port.h
+++ b/include/gtest/internal/gtest-port.h
@@ -1838,8 +1838,9 @@ class ThreadWithParam : public ThreadWithParamBase {
 template <typename T>
 class ThreadLocal : public ThreadLocalBase {
  public:
-  ThreadLocal() : default_() {}
-  explicit ThreadLocal(const T& value) : default_(value) {}
+  ThreadLocal() : default_factory_(new DefaultValueHolderFactory()) {}
+  explicit ThreadLocal(const T& value)
+      : default_factory_(new InstanceValueHolderFactory(value)) {}
 
   ~ThreadLocal() { ThreadLocalRegistry::OnThreadLocalDestroyed(this); }
 
@@ -1853,6 +1854,7 @@ class ThreadLocal : public ThreadLocalBase {
   // knowing the type of T.
   class ValueHolder : public ThreadLocalValueHolderBase {
    public:
+    ValueHolder() : value_() {}
     explicit ValueHolder(const T& value) : value_(value) {}
 
     T* pointer() { return &value_; }
@@ -1869,10 +1871,42 @@ class ThreadLocal : public ThreadLocalBase {
   }
 
   virtual ThreadLocalValueHolderBase* NewValueForCurrentThread() const {
-    return new ValueHolder(default_);
+    return default_factory_->MakeNewHolder();
   }
 
-  const T default_;  // The default value for each thread.
+  class ValueHolderFactory {
+   public:
+    ValueHolderFactory() {}
+    virtual ~ValueHolderFactory() {}
+    virtual ValueHolder* MakeNewHolder() const = 0;
+
+   private:
+    GTEST_DISALLOW_COPY_AND_ASSIGN_(ValueHolderFactory);
+  };
+
+  class DefaultValueHolderFactory : public ValueHolderFactory {
+   public:
+    DefaultValueHolderFactory() {}
+    virtual ValueHolder* MakeNewHolder() const { return new ValueHolder(); }
+
+   private:
+    GTEST_DISALLOW_COPY_AND_ASSIGN_(DefaultValueHolderFactory);
+  };
+
+  class InstanceValueHolderFactory : public ValueHolderFactory {
+   public:
+    explicit InstanceValueHolderFactory(const T& value) : value_(value) {}
+    virtual ValueHolder* MakeNewHolder() const {
+      return new ValueHolder(value_);
+    }
+
+   private:
+    const T value_;  // The value for each thread.
+
+    GTEST_DISALLOW_COPY_AND_ASSIGN_(InstanceValueHolderFactory);
+  };
+
+  scoped_ptr<ValueHolderFactory> default_factory_;
 
   GTEST_DISALLOW_COPY_AND_ASSIGN_(ThreadLocal);
 };
@@ -1993,10 +2027,11 @@ extern "C" inline void DeleteThreadLocalValue(void* value_holder) {
 template <typename T>
 class ThreadLocal {
  public:
-  ThreadLocal() : key_(CreateKey()),
-                  default_() {}
-  explicit ThreadLocal(const T& value) : key_(CreateKey()),
-                                         default_(value) {}
+  ThreadLocal()
+      : key_(CreateKey()), default_factory_(new DefaultValueHolderFactory()) {}
+  explicit ThreadLocal(const T& value)
+      : key_(CreateKey()),
+        default_factory_(new InstanceValueHolderFactory(value)) {}
 
   ~ThreadLocal() {
     // Destroys the managed object for the current thread, if any.
@@ -2016,6 +2051,7 @@ class ThreadLocal {
   // Holds a value of type T.
   class ValueHolder : public ThreadLocalValueHolderBase {
    public:
+    ValueHolder() : value_() {}
     explicit ValueHolder(const T& value) : value_(value) {}
 
     T* pointer() { return &value_; }
@@ -2041,15 +2077,47 @@ class ThreadLocal {
       return CheckedDowncastToActualType<ValueHolder>(holder)->pointer();
     }
 
-    ValueHolder* const new_holder = new ValueHolder(default_);
+    ValueHolder* const new_holder = default_factory_->MakeNewHolder();
     ThreadLocalValueHolderBase* const holder_base = new_holder;
     GTEST_CHECK_POSIX_SUCCESS_(pthread_setspecific(key_, holder_base));
     return new_holder->pointer();
   }
 
+  class ValueHolderFactory {
+   public:
+    ValueHolderFactory() {}
+    virtual ~ValueHolderFactory() {}
+    virtual ValueHolder* MakeNewHolder() const = 0;
+
+   private:
+    GTEST_DISALLOW_COPY_AND_ASSIGN_(ValueHolderFactory);
+  };
+
+  class DefaultValueHolderFactory : public ValueHolderFactory {
+   public:
+    DefaultValueHolderFactory() {}
+    virtual ValueHolder* MakeNewHolder() const { return new ValueHolder(); }
+
+   private:
+    GTEST_DISALLOW_COPY_AND_ASSIGN_(DefaultValueHolderFactory);
+  };
+
+  class InstanceValueHolderFactory : public ValueHolderFactory {
+   public:
+    explicit InstanceValueHolderFactory(const T& value) : value_(value) {}
+    virtual ValueHolder* MakeNewHolder() const {
+      return new ValueHolder(value_);
+    }
+
+   private:
+    const T value_;  // The value for each thread.
+
+    GTEST_DISALLOW_COPY_AND_ASSIGN_(InstanceValueHolderFactory);
+  };
+
   // A key pthreads uses for looking up per-thread values.
   const pthread_key_t key_;
-  const T default_;  // The default value for each thread.
+  scoped_ptr<ValueHolderFactory> default_factory_;
 
   GTEST_DISALLOW_COPY_AND_ASSIGN_(ThreadLocal);
 };
diff --git a/test/gtest-port_test.cc b/test/gtest-port_test.cc
index 937832b..1441880 100644
--- a/test/gtest-port_test.cc
+++ b/test/gtest-port_test.cc
@@ -1149,13 +1149,6 @@ TEST(ThreadLocalTest, ParameterizedConstructorSetsDefault) {
   EXPECT_STREQ("foo", result.c_str());
 }
 
-# if !GTEST_HAS_MUTEX_AND_THREAD_LOCAL_
-
-// Tests in this section depend on that Google Test's own ThreadLocal
-// implementation stores a copy of the default value shared by all
-// threads.  We don't want to test this for an external implementation received
-// through GTEST_HAS_MUTEX_AND_THREAD_LOCAL_.
-
 // Keeps track of whether of destructors being called on instances of
 // DestructorTracker.  On Windows, waits for the destructor call reports.
 class DestructorCall {
@@ -1240,25 +1233,18 @@ TEST(ThreadLocalTest, DestroysManagedObjectForOwnThreadWhenDying) {
   DestructorCall::ResetList();
 
   {
-    // The next line default constructs a DestructorTracker object as
-    // the default value of objects managed by thread_local_tracker.
     ThreadLocal<DestructorTracker> thread_local_tracker;
-    ASSERT_EQ(1U, DestructorCall::List().size());
-    ASSERT_FALSE(DestructorCall::List()[0]->CheckDestroyed());
+    ASSERT_EQ(0U, DestructorCall::List().size());
 
     // This creates another DestructorTracker object for the main thread.
     thread_local_tracker.get();
-    ASSERT_EQ(2U, DestructorCall::List().size());
+    ASSERT_EQ(1U, DestructorCall::List().size());
     ASSERT_FALSE(DestructorCall::List()[0]->CheckDestroyed());
-    ASSERT_FALSE(DestructorCall::List()[1]->CheckDestroyed());
   }
 
-  // Now thread_local_tracker has died.  It should have destroyed both the
-  // default value shared by all threads and the value for the main
-  // thread.
-  ASSERT_EQ(2U, DestructorCall::List().size());
+  // Now thread_local_tracker has died.
+  ASSERT_EQ(1U, DestructorCall::List().size());
   EXPECT_TRUE(DestructorCall::List()[0]->CheckDestroyed());
-  EXPECT_TRUE(DestructorCall::List()[1]->CheckDestroyed());
 
   DestructorCall::ResetList();
 }
@@ -1269,35 +1255,26 @@ TEST(ThreadLocalTest, DestroysManagedObjectAtThreadExit) {
   DestructorCall::ResetList();
 
   {
-    // The next line default constructs a DestructorTracker object as
-    // the default value of objects managed by thread_local_tracker.
     ThreadLocal<DestructorTracker> thread_local_tracker;
-    ASSERT_EQ(1U, DestructorCall::List().size());
-    ASSERT_FALSE(DestructorCall::List()[0]->CheckDestroyed());
+    ASSERT_EQ(0U, DestructorCall::List().size());
 
     // This creates another DestructorTracker object in the new thread.
     ThreadWithParam<ThreadParam> thread(
         &CallThreadLocalGet, &thread_local_tracker, NULL);
     thread.Join();
 
-    // The thread has exited, and we should have another DestroyedTracker
+    // The thread has exited, and we should have a DestroyedTracker
     // instance created for it. But it may not have been destroyed yet.
-    // The instance for the main thread should still persist.
-    ASSERT_EQ(2U, DestructorCall::List().size());
-    ASSERT_FALSE(DestructorCall::List()[0]->CheckDestroyed());
+    ASSERT_EQ(1U, DestructorCall::List().size());
   }
 
-  // The thread has exited and thread_local_tracker has died.  The default
-  // value should have been destroyed too.
-  ASSERT_EQ(2U, DestructorCall::List().size());
+  // The thread has exited and thread_local_tracker has died.
+  ASSERT_EQ(1U, DestructorCall::List().size());
   EXPECT_TRUE(DestructorCall::List()[0]->CheckDestroyed());
-  EXPECT_TRUE(DestructorCall::List()[1]->CheckDestroyed());
 
   DestructorCall::ResetList();
 }
 
-# endif  // !GTEST_HAS_MUTEX_AND_THREAD_LOCAL_
-
 TEST(ThreadLocalTest, ThreadLocalMutationsAffectOnlyCurrentThread) {
   ThreadLocal<std::string> thread_local_string;
   thread_local_string.set("Foo");