Wait for sampling to complete before destructing.

base/profiler/stack_sampling_profiler.cc

Index: base/profiler/stack_sampling_profiler.cc
diff --git a/base/profiler/stack_sampling_profiler.cc b/base/profiler/stack_sampling_profiler.cc
index ce898d4c07f7a797455419f1e7bd2e64fdf05581..91cc193f23d77dcf256578df0695a1e1bb48b47d 100644
--- a/base/profiler/stack_sampling_profiler.cc
+++ b/base/profiler/stack_sampling_profiler.cc
@@ -28,63 +28,6 @@ namespace base {
 
 namespace {
 
-// AsyncRunner ----------------------------------------------------------------
-
-// Helper class to allow a profiler to be run completely asynchronously from the
-// initiator, without being concerned with the profiler's lifetime.
-class AsyncRunner {
- public:
-  // Sets up a profiler and arranges for it to be deleted on its completed
-  // callback.
-  static void Run(PlatformThreadId thread_id,
-                  const StackSamplingProfiler::SamplingParams& params,
-                  const StackSamplingProfiler::CompletedCallback& callback);
-
- private:
-  AsyncRunner();
-
-  // Runs the callback and deletes the AsyncRunner instance. |profiles| is not
-  // const& because it must be passed with std::move.
-  static void RunCallbackAndDeleteInstance(
-      std::unique_ptr<AsyncRunner> object_to_be_deleted,
-      const StackSamplingProfiler::CompletedCallback& callback,
-      scoped_refptr<SingleThreadTaskRunner> task_runner,
-      StackSamplingProfiler::CallStackProfiles profiles);
-
-  std::unique_ptr<StackSamplingProfiler> profiler_;
-
-  DISALLOW_COPY_AND_ASSIGN(AsyncRunner);
-};
-
-// static
-void AsyncRunner::Run(
-    PlatformThreadId thread_id,
-    const StackSamplingProfiler::SamplingParams& params,
-    const StackSamplingProfiler::CompletedCallback &callback) {
-  std::unique_ptr<AsyncRunner> runner(new AsyncRunner);
-  AsyncRunner* temp_ptr = runner.get();
-  temp_ptr->profiler_.reset(
-      new StackSamplingProfiler(thread_id, params,
-                                Bind(&AsyncRunner::RunCallbackAndDeleteInstance,
-                                     Passed(&runner), callback,
-                                     ThreadTaskRunnerHandle::Get())));
-  // The callback won't be called until after Start(), so temp_ptr will still
-  // be valid here.
-  temp_ptr->profiler_->Start();
-}
-
-AsyncRunner::AsyncRunner() {}
-
-void AsyncRunner::RunCallbackAndDeleteInstance(
-    std::unique_ptr<AsyncRunner> object_to_be_deleted,
-    const StackSamplingProfiler::CompletedCallback& callback,
-    scoped_refptr<SingleThreadTaskRunner> task_runner,
-    StackSamplingProfiler::CallStackProfiles profiles) {
-  callback.Run(std::move(profiles));
-  // Delete the instance on the original calling thread.
-  task_runner->DeleteSoon(FROM_HERE, object_to_be_deleted.release());
-}
-
 void ChangeAtomicFlags(subtle::Atomic32* flags,
                        subtle::Atomic32 set,
                        subtle::Atomic32 clear) {
@@ -216,8 +159,9 @@ class StackSamplingProfiler::SamplingThread : public Thread {
 
   // Removes an active collection based on its ID, forcing it to run its
   // callback if any data has been collected. This can be called externally
-  // from any thread.
-  void Remove(int id);
+  // from any thread. The |done| event, if not null, will be signaled upon
+  // completion. |done| must live until after it has been signaled.
+  void Remove(int id, WaitableEvent* done);
 
   // Removes all active collections and stops the underlying thread.
   void Shutdown();
@@ -255,7 +199,7 @@ class StackSamplingProfiler::SamplingThread : public Thread {
 
   // These methods are tasks that get posted to the internal message queue.
   void AddCollectionTask(std::unique_ptr<CollectionContext> collection_ptr);
-  void RemoveCollectionTask(int id);
+  void RemoveCollectionTask(int id, WaitableEvent* done);
   void PerformCollectionTask(int id);
   void ShutdownTask();
 
@@ -320,16 +264,24 @@ int StackSamplingProfiler::SamplingThread::Add(
   return id;
 }
 
-void StackSamplingProfiler::SamplingThread::Remove(int id) {
+void StackSamplingProfiler::SamplingThread::Remove(int id,
+                                                   WaitableEvent* done) {
   scoped_refptr<SingleThreadTaskRunner> task_runner = GetTaskRunner();
-  if (!task_runner)
-    return;  // Everything has already stopped.
+  if (task_runner) {
+    // This can fail if the thread were to exit between acquisition of the task
+    // runner above and the call below. Because there may be something waiting
+    // for a signal on the |done| event, return only if the task was success-
+    // fully posted.
+    if (task_runner->PostTask(FROM_HERE,
+                              Bind(&SamplingThread::RemoveCollectionTask,
+                                   Unretained(this), id, Unretained(done)))) {
+      return;
+    }
+  }
 
-  // This can fail if the thread were to exit between acquisition of the task
-  // runner above and the call below. In that case, however, everything has
-  // stopped so there's no need to try to stop it.
-  task_runner->PostTask(FROM_HERE, Bind(&SamplingThread::RemoveCollectionTask,
-                                        Unretained(this), id));
+  // The sampling thread has already exited so signal the event here.
+  if (done)
+    done->Signal();
 }
 
 void StackSamplingProfiler::SamplingThread::Shutdown() {
@@ -514,13 +466,17 @@ void StackSamplingProfiler::SamplingThread::AddCollectionTask(
       collection->params.initial_delay);
 }
 
-void StackSamplingProfiler::SamplingThread::RemoveCollectionTask(int id) {
+void StackSamplingProfiler::SamplingThread::RemoveCollectionTask(
+    int id,
+    WaitableEvent* done) {
   auto found = active_collections_.find(id);
-  if (found == active_collections_.end())
-    return;
+  if (found != active_collections_.end()) {
+    FinishCollection(found->second.get());
+    CheckForIdle();
+  }
 
-  FinishCollection(found->second.get());
-  CheckForIdle();
+  if (done)
+    done->Signal();
 }
 
 void StackSamplingProfiler::SamplingThread::PerformCollectionTask(int id) {
@@ -658,31 +614,10 @@ StackSamplingProfiler::StackSamplingProfiler(
 }
 
 StackSamplingProfiler::~StackSamplingProfiler() {
-  Stop();
-
-#if !defined(OS_WIN)
-  // This is to prevent use of this object on non-Windows builds until such
-  // time as the following race condition is addressed:
-  //   Thread A creates a local (stack) StackSamplingProfiler.
-  //   Sampling is initiated against thread A ("A samples A").
-  //   The StackSamplingProfiler is destructed and thread A exits.
-  //   Sampling continues briefly until the async Stop() gets through.
-  //   Another thread with the same ID replaces the first one.
-  //   Sampling access the new thread and Bad Things(tm) happen.
-  // Windows is immune to this because the thread handle held internally
-  // prevents re-use of the thread-ID but this may not be the case on other
-  // platforms.
-  NOTREACHED();
-#endif
-}
-
-// static
-void StackSamplingProfiler::StartAndRunAsync(
-    PlatformThreadId thread_id,
-    const SamplingParams& params,
-    const CompletedCallback& callback) {
-  CHECK(ThreadTaskRunnerHandle::Get());
-  AsyncRunner::Run(thread_id, params, callback);
+  WaitableEvent done(WaitableEvent::ResetPolicy::MANUAL,
+                     WaitableEvent::InitialState::NOT_SIGNALED);
+  Stop(&done);
+  done.Wait();
 }
 
 void StackSamplingProfiler::Start() {
@@ -698,8 +633,8 @@ void StackSamplingProfiler::Start() {
           std::move(native_sampler_)));
 }
 
-void StackSamplingProfiler::Stop() {
-  SamplingThread::GetInstance()->Remove(collection_id_);
+void StackSamplingProfiler::Stop(WaitableEvent* done) {
+  SamplingThread::GetInstance()->Remove(collection_id_, done);
 }
 
 // static