Adds a way to associate key/value pairs with the environment

This is to fix a deadlock during shutdown. Specifically
WatcherThreadManager was a LazyInstance. This means it would be
shutdown by LazyInstance, which holds a lock. If the thread attempted
to grab the lazyinstance lock (which it does), then we deadlock.

Making the WatcherTheadManager owned by Environment makes for saner
lifetime management and easier shutdown in tests.

BUG=none
TEST=none
R=darin@chromium.org

Review URL: https://codereview.chromium.org/218583009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@261045 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 141 insertions, 10 deletions

diff --git a/mojo/common/environment_data.cc b/mojo/common/environment_data.cc
new file mode 100644
index 0000000..b523d27
--- /dev/null
+++ b/mojo/common/environment_data.cc
@@ -0,0 +1,51 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "mojo/common/environment_data.h"
+
+#include "base/stl_util.h"
+
+namespace mojo {
+namespace common {
+
+// static
+EnvironmentData* EnvironmentData::instance_ = NULL;
+
+EnvironmentData::EnvironmentData() {
+  DCHECK(!instance_);
+  instance_ = this;
+}
+
+EnvironmentData::~EnvironmentData() {
+  instance_ = NULL;
+  DataMap data_map;
+  data_map.swap(data_map_);
+  STLDeleteContainerPairSecondPointers(data_map.begin(), data_map.end());
+}
+
+// static
+EnvironmentData* EnvironmentData::GetInstance() {
+  return instance_;
+}
+
+void EnvironmentData::SetData(const void* key, scoped_ptr<Data> data) {
+  Data* old = NULL;
+  {
+    base::AutoLock auto_lock(data_lock_);
+    old = data_map_[key];
+    if (data)
+      data_map_[key] = data.release();
+    else
+      data_map_.erase(key);
+  }
+  delete old;
+}
+
+EnvironmentData::Data* EnvironmentData::GetData(const void* key) {
+  base::AutoLock auto_lock(data_lock_);
+  return data_map_.count(key) > 0 ? data_map_[key] : NULL;
+}
+
+}  // namespace common
+}  // namespace mojo
diff --git a/mojo/common/environment_data.h b/mojo/common/environment_data.h
new file mode 100644
index 0000000..4440df2
--- /dev/null
+++ b/mojo/common/environment_data.h
@@ -0,0 +1,52 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef MOJO_COMMON_ENVIRONMENT_DATA_H_
+#define MOJO_COMMON_ENVIRONMENT_DATA_H_
+
+#include <map>
+
+#include "base/memory/scoped_ptr.h"
+#include "base/synchronization/lock.h"
+#include "mojo/common/mojo_common_export.h"
+
+namespace mojo {
+namespace common {
+
+// EnvironmentData is used to store arbitrary key/value pairs in the
+// environment. The key/value pairs are owned by the Environment and deleted
+// when it is deleted.
+class MOJO_COMMON_EXPORT EnvironmentData {
+ public:
+  class MOJO_COMMON_EXPORT Data {
+   public:
+    Data() {}
+    virtual ~Data() {}
+  };
+
+  EnvironmentData();
+  ~EnvironmentData();
+
+  static EnvironmentData* GetInstance();
+
+  void SetData(const void* key, scoped_ptr<Data> data);
+
+  Data* GetData(const void* key);
+
+ private:
+  typedef std::map<const void*, Data*> DataMap;
+
+  static EnvironmentData* instance_;
+
+  base::Lock data_lock_;
+
+  DataMap data_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(EnvironmentData);
+};
+
+}  // namespace common
+}  // namespace mojo
+
+#endif  // MOJO_COMMON_ENVIRONMENT_DATA_H_
diff --git a/mojo/common/handle_watcher.cc b/mojo/common/handle_watcher.cc
index 150305c..a95669c 100644
--- a/mojo/common/handle_watcher.cc
+++ b/mojo/common/handle_watcher.cc
@@ -12,8 +12,10 @@
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "base/synchronization/lock.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
+#include "mojo/common/environment_data.h"
 #include "mojo/common/message_pump_mojo.h"
 #include "mojo/common/message_pump_mojo_handler.h"
 #include "mojo/common/time_helper.h"
@@ -27,6 +29,8 @@ namespace {
 
 const char kWatcherThreadName[] = "handle-watcher-thread";
 
+const char kWatcherThreadManagerKey[] = "watcher-thread-manager";
+
 // TODO(sky): this should be unnecessary once MessageLoop has been refactored.
 MessagePumpMojo* message_pump_mojo = NULL;
 
@@ -153,6 +157,8 @@ void WatcherBackend::OnHandleError(const Handle& handle, MojoResult result) {
 // to be ready. All requests are handled by WatcherBackend.
 class WatcherThreadManager {
  public:
+  ~WatcherThreadManager();
+
   // Returns the shared instance.
   static WatcherThreadManager* GetInstance();
 
@@ -170,10 +176,7 @@ class WatcherThreadManager {
   void StopWatching(WatcherID watcher_id);
 
  private:
-  friend struct base::DefaultLazyInstanceTraits<WatcherThreadManager>;
-
   WatcherThreadManager();
-  ~WatcherThreadManager();
 
   base::Thread thread_;
 
@@ -184,10 +187,29 @@ class WatcherThreadManager {
   DISALLOW_COPY_AND_ASSIGN(WatcherThreadManager);
 };
 
+struct WatcherThreadManagerData : EnvironmentData::Data {
+  scoped_ptr<WatcherThreadManager> thread_manager;
+};
+
+WatcherThreadManager::~WatcherThreadManager() {
+  thread_.Stop();
+}
+
+static base::LazyInstance<base::Lock> thread_lookup_lock =
+    LAZY_INSTANCE_INITIALIZER;
+
 WatcherThreadManager* WatcherThreadManager::GetInstance() {
-  static base::LazyInstance<WatcherThreadManager> instance =
-      LAZY_INSTANCE_INITIALIZER;
-  return &instance.Get();
+  base::AutoLock auto_lock(thread_lookup_lock.Get());
+  WatcherThreadManagerData* data = static_cast<WatcherThreadManagerData*>(
+      EnvironmentData::GetInstance()->GetData(kWatcherThreadManagerKey));
+  if (!data) {
+    data = new WatcherThreadManagerData;
+    data->thread_manager.reset(new WatcherThreadManager);
+    EnvironmentData::GetInstance()->SetData(
+        kWatcherThreadManagerKey,
+        scoped_ptr<EnvironmentData::Data>(data));
+  }
+  return data->thread_manager.get();
 }
 
 WatcherID WatcherThreadManager::StartWatching(
@@ -229,10 +251,6 @@ WatcherThreadManager::WatcherThreadManager()
   thread_.StartWithOptions(thread_options);
 }
 
-WatcherThreadManager::~WatcherThreadManager() {
-  thread_.Stop();
-}
-
 }  // namespace
 
 // HandleWatcher::StartState ---------------------------------------------------
diff --git a/mojo/common/handle_watcher_unittest.cc b/mojo/common/handle_watcher_unittest.cc
index 729ffe1..5cc8cfa 100644
--- a/mojo/common/handle_watcher_unittest.cc
+++ b/mojo/common/handle_watcher_unittest.cc
@@ -11,6 +11,7 @@
 #include "base/run_loop.h"
 #include "base/test/simple_test_tick_clock.h"
 #include "mojo/common/time_helper.h"
+#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/cpp/system/core.h"
 #include "mojo/public/cpp/test_support/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -100,6 +101,14 @@ class HandleWatcherTest : public testing::Test {
     test::SetTickClockForTest(NULL);
   }
 
+  virtual void SetUp() OVERRIDE {
+    environment_.reset(new Environment);
+  }
+
+  virtual void TearDown() OVERRIDE {
+    environment_.reset();
+  }
+
  protected:
   void InstallTickClock() {
     test::SetTickClockForTest(&tick_clock_);
@@ -109,6 +118,7 @@ class HandleWatcherTest : public testing::Test {
 
  private:
   base::MessageLoop message_loop_;
+  scoped_ptr<Environment> environment_;
 
   DISALLOW_COPY_AND_ASSIGN(HandleWatcherTest);
 };